| Message ID | 20241101-asahi-spi-v3-0-3b411c5fb8e5@jannau.net |
|---|---|
| Headers | show |
| Series | Apple SPI controller driver | expand |
On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote: [...] > +properties: > + compatible: > + items: > + - enum: > + - apple,t8103-spi > + - apple,t8112-spi > + - apple,t6000-spi > + - const: apple,spi Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too generic. Fallback to something like apple,t8103-spi instead. [...] Nick Chan
On Sat, Nov 02, 2024 at 10:36:56AM +0800, Nick Chan wrote: > > > On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote: > > [...] > > +properties: > > + compatible: > > + items: > > + - enum: > > + - apple,t8103-spi > > + - apple,t8112-spi > > + - apple,t6000-spi > > + - const: apple,spi > Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too > generic. Fallback to something like apple,t8103-spi instead. Have you looked at it? This one still seems to be somehow Samsung related. See the previous discussion at https://lore.kernel.org/linux-devicetree/d87ae109-4b58-7465-b16e-3bf7c9d60f1f@marcan.st/ Even the A7-A11 SPI controllers are not compatible "apple,spi" doesn't prevent us from using something like "apple,s5l-spi" for those. Janne
> Date: Sat, 2 Nov 2024 10:36:56 +0800 > Content-Language: en-MW > > On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote: > > [...] > > +properties: > > + compatible: > > + items: > > + - enum: > > + - apple,t8103-spi > > + - apple,t8112-spi > > + - apple,t6000-spi > > + - const: apple,spi > Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too > generic. Fallback to something like apple,t8103-spi instead. Well, if that is a Samsung SPI block it probably should have a "generic" compatible that starts with "samsung,". The M1/M2 controllers have a different SPI block (presumably) designed by Apple themselves. So I think it is (still) appropriate that that one is "apple,spi". Also, (upstream) U-Boot already uses the "apple,spi" compatible. So changing it for purity sake just causes pain.
On Fri, Nov 01, 2024 at 08:26:11PM +0100, Janne Grunau wrote: > Hi all, > > This updated series address the review comments from the original > submission in 2021 [1]. It adds a new SPI controller driver for Apple > SoCs and is based on spi-sifive. It has been tested with the generic > jedec,spi-nor support and with a downstream driver for an Apple specific > HID over SPI transport. > > As usual, I'm splitting off the MAINTAINERS and DT binding changes. > We would rather merge the MAINTAINERS change through the Asahi-SoC > tree to avoid merge conflicts as things trickle upstream, since > we have other submissions touching that section of the file. > > The DT binding change can go via the SPI tree or via ours, but it's > easier if we merge it, as then we can make the DT changes to > instantiate it without worrying about DT validation failures depending > on merge order. > > This is mostly Hector's work with a few minor changes to address review > comments from me. > > [1] https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ > > v2: > - removed '#address-cells' and '#size-cells' from the bindings and added > Rob's Rb: Where? Best regards, Krzysztof
On 2/11/2024 16:39, Mark Kettenis wrote: >> Date: Sat, 2 Nov 2024 10:36:56 +0800 >> Content-Language: en-MW >> >> On 2/11/2024 03:26, Janne Grunau via B4 Relay wrote: >> >> [...] >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - apple,t8103-spi >>> + - apple,t8112-spi >>> + - apple,t6000-spi >>> + - const: apple,spi >> Apple A7-A11 SoCs seems to use a Samsung SPI block, so apple,spi is too >> generic. Fallback to something like apple,t8103-spi instead. > > Well, if that is a Samsung SPI block it probably should have a > "generic" compatible that starts with "samsung,". The M1/M2 > controllers have a different SPI block (presumably) designed by Apple > themselves. So I think it is (still) appropriate that that one is > "apple,spi". I just looked into the SPI on A7-A11 SoC in more detail instead of just going off the ADT compatible. It seems a very big chunk of the registers offsets and bits seems to be the same as the ones in M1. So, feel free to ignore my comment above. Acked-by: Nick Chan <towinchenmi@gmail.com> > > Also, (upstream) U-Boot already uses the "apple,spi" compatible. So > changing it for purity sake just causes pain. Well, if upstream U-Boot is using it, then I agree that "apple,spi" should continue to be used. > Nick Chan
On Sat, Nov 02, 2024 at 02:11:51PM +0100, Krzysztof Kozlowski wrote: > On Fri, Nov 01, 2024 at 08:26:11PM +0100, Janne Grunau wrote: > > Hi all, > > > > This updated series address the review comments from the original > > submission in 2021 [1]. It adds a new SPI controller driver for Apple > > SoCs and is based on spi-sifive. It has been tested with the generic > > jedec,spi-nor support and with a downstream driver for an Apple specific > > HID over SPI transport. > > > > As usual, I'm splitting off the MAINTAINERS and DT binding changes. > > We would rather merge the MAINTAINERS change through the Asahi-SoC > > tree to avoid merge conflicts as things trickle upstream, since > > we have other submissions touching that section of the file. > > > > The DT binding change can go via the SPI tree or via ours, but it's > > easier if we merge it, as then we can make the DT changes to > > instantiate it without worrying about DT validation failures depending > > on merge order. > > > > This is mostly Hector's work with a few minor changes to address review > > comments from me. > > > > [1] https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ > > > > v2: > > - removed '#address-cells' and '#size-cells' from the bindings and added > > Rob's Rb: > > Where? Apparently only in my mind. fixed for v4 thanks, Janne
Hi all, This updated series address the review comments from the original submission in 2021 [1]. It adds a new SPI controller driver for Apple SoCs and is based on spi-sifive. It has been tested with the generic jedec,spi-nor support and with a downstream driver for an Apple specific HID over SPI transport. As usual, I'm splitting off the MAINTAINERS and DT binding changes. We would rather merge the MAINTAINERS change through the Asahi-SoC tree to avoid merge conflicts as things trickle upstream, since we have other submissions touching that section of the file. The DT binding change can go via the SPI tree or via ours, but it's easier if we merge it, as then we can make the DT changes to instantiate it without worrying about DT validation failures depending on merge order. This is mostly Hector's work with a few minor changes to address review comments from me. [1] https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ v2: - removed '#address-cells' and '#size-cells' from the bindings and added Rob's Rb: - fixed (new) checkpatch warnings - added t8112 (M2) SoC - shorted long and complex source code lines - switch to devm_clk_prepare_enable() and devm_pm_runtime_enable() - switch to dev_err_probe() in probe function - removed "pdev->dev.dma_mask = NULL;" - got rid of apple_spi_remove() Signed-off-by: Janne Grunau <j@jannau.net> --- Changes in v3: - fixed bindings_check warning - converted top file comment to C++ style comments - dropped verbose dev_err_probe after failed devm_* function - stopped setting field in zero initialized struct to 0 - added error handling for devm_pm_runtime_enable() - Link to v2: https://lore.kernel.org/r/20241101-asahi-spi-v2-0-763a8a84d834@jannau.net Changes in v2: - removed '#address-cells' and '#size-cells' from the bindings and added Rob's Rb: - fixed (new) checkpatch warnings - added t8112 (M2) SoC - shorted long and complex source code lines - switch to devm_clk_prepare_enable() and devm_pm_runtime_enable() - switch to dev_err_probe() in probe function - removed "pdev->dev.dma_mask = NULL;" - got rid of apple_spi_remove() - Link to v1: https://lore.kernel.org/linux-spi/20211212034726.26306-1-marcan@marcan.st/ --- Hector Martin (3): dt-bindings: spi: apple,spi: Add binding for Apple SPI controllers spi: apple: Add driver for Apple SPI controller MAINTAINERS: Add apple-spi driver & binding files .../devicetree/bindings/spi/apple,spi.yaml | 62 +++ MAINTAINERS | 2 + drivers/spi/Kconfig | 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-apple.c | 531 +++++++++++++++++++++ 5 files changed, 607 insertions(+) --- base-commit: 98f7e32f20d28ec452afb208f9cffc08448a2652 change-id: 20240811-asahi-spi-f8740ba797d7 Best regards,