| Message ID | 1563393026-17118-10-git-send-email-wahrenst@gmx.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: Add minimal Raspberry Pi 4 support | expand |
On 17/07/2019 21:50, Stefan Wahren wrote: > The additional emmc2 interface of the BCM2838 is an improved version > of the old emmc controller, which is able to provide DDR50 mode on the > Raspberry Pi 4. Except 32 bit only register access no other quirks are > known yet. > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > --- > drivers/mmc/host/sdhci-iproc.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index 2feb4ef..353cf997 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -261,8 +261,17 @@ static const struct sdhci_iproc_data bcm2835_data = { > .mmc_caps = 0x00000000, > }; > > +static const struct sdhci_pltfm_data sdhci_bcm2838_pltfm_data = { > + .ops = &sdhci_iproc_32only_ops, > +}; > + > +static const struct sdhci_iproc_data bcm2838_data = { > + .pdata = &sdhci_bcm2838_pltfm_data, > +}; > + > static const struct of_device_id sdhci_iproc_of_match[] = { > { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, > + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, As far as I'm aware of, the RPi4 FW provides a device-tree with compatible: brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can use the DT passed by the FW? Regards, Matthias > { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data}, > { .compatible = "brcm,sdhci-iproc", .data = &iproc_data }, > { } > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 7/18/2019 1:34 AM, Matthias Brugger wrote: [snip] >> static const struct of_device_id sdhci_iproc_of_match[] = { >> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, >> + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, > > As far as I'm aware of, the RPi4 FW provides a device-tree with compatible: > brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can use the DT > passed by the FW? Downstream typically used 2708, 2709, 2710 because those are the Broadcom internal part numbers, and upstream has been using what's on the package: 2835, 2836, 2837, 2838. At the end of the day, it does not make much functional difference, but if if we have to be consistent, then Stefan's approach here follows the consistency here.
On 18/07/2019 18:40, Florian Fainelli wrote: > > > On 7/18/2019 1:34 AM, Matthias Brugger wrote: > > [snip] > >>> static const struct of_device_id sdhci_iproc_of_match[] = { >>> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, >>> + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, >> >> As far as I'm aware of, the RPi4 FW provides a device-tree with compatible: >> brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can use the DT >> passed by the FW? > > Downstream typically used 2708, 2709, 2710 because those are the > Broadcom internal part numbers, and upstream has been using what's on > the package: 2835, 2836, 2837, 2838. At the end of the day, it does not > make much functional difference, but if if we have to be consistent, > then Stefan's approach here follows the consistency here. > So I propose to add both, so that we can use the upstream kernel with downstream devcie-tree. I'm thinking of the device-tree provided at run-time by the FW. Regards, Matthias
On 7/18/2019 9:48 AM, Matthias Brugger wrote: > > > On 18/07/2019 18:40, Florian Fainelli wrote: >> >> >> On 7/18/2019 1:34 AM, Matthias Brugger wrote: >> >> [snip] >> >>>> static const struct of_device_id sdhci_iproc_of_match[] = { >>>> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, >>>> + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, >>> >>> As far as I'm aware of, the RPi4 FW provides a device-tree with compatible: >>> brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can use the DT >>> passed by the FW? >> >> Downstream typically used 2708, 2709, 2710 because those are the >> Broadcom internal part numbers, and upstream has been using what's on >> the package: 2835, 2836, 2837, 2838. At the end of the day, it does not >> make much functional difference, but if if we have to be consistent, >> then Stefan's approach here follows the consistency here. >> > > So I propose to add both, so that we can use the upstream kernel with downstream > devcie-tree. I'm thinking of the device-tree provided at run-time by the FW. Adding both for the Pi4 (2711) specifically, or should we go back and also add bcm2708/9/10? The Device Tree on the Pi can be updated (AFAICT), so the ABI concerns, and the requirement for upstream to maintain backwards compatibility with a binding that has not been submitted/reviewed is not going to be a compelling argument IMHO.
Am 18.07.19 um 18:48 schrieb Matthias Brugger: > > On 18/07/2019 18:40, Florian Fainelli wrote: >> >> On 7/18/2019 1:34 AM, Matthias Brugger wrote: >> >> [snip] >> >>>> static const struct of_device_id sdhci_iproc_of_match[] = { >>>> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, >>>> + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, >>> As far as I'm aware of, the RPi4 FW provides a device-tree with compatible: >>> brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can use the DT >>> passed by the FW? >> Downstream typically used 2708, 2709, 2710 because those are the >> Broadcom internal part numbers, and upstream has been using what's on >> the package: 2835, 2836, 2837, 2838. At the end of the day, it does not >> make much functional difference, but if if we have to be consistent, >> then Stefan's approach here follows the consistency here. >> > So I propose to add both, so that we can use the upstream kernel with downstream > devcie-tree. I'm thinking of the device-tree provided at run-time by the FW. AFAIK the FW doesn't know anything about the devicetree part of the emmc2. So i suggest to add the upstream compatible in the downstream tree. This way we keep out all the downstream stuff (as before) and avoid confusion in the devicetree bindings. Stefan > > Regards, > Matthias > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Matthias, Am 18.07.19 um 18:48 schrieb Matthias Brugger: > > On 18/07/2019 18:40, Florian Fainelli wrote: >> >> On 7/18/2019 1:34 AM, Matthias Brugger wrote: >> >> [snip] >> >>>> static const struct of_device_id sdhci_iproc_of_match[] = { >>>> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, >>>> + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, >>> As far as I'm aware of, the RPi4 FW provides a device-tree with compatible: >>> brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can use the DT >>> passed by the FW? >> Downstream typically used 2708, 2709, 2710 because those are the >> Broadcom internal part numbers, and upstream has been using what's on >> the package: 2835, 2836, 2837, 2838. At the end of the day, it does not >> make much functional difference, but if if we have to be consistent, >> then Stefan's approach here follows the consistency here. >> > So I propose to add both, so that we can use the upstream kernel with downstream > devcie-tree. I'm thinking of the device-tree provided at run-time by the FW. sorry for the second mail, but this is a slightly different topic. I've seen that Andrei already submitted a patch series for u-boot with a different DTS file. In the past, we had the following workflow for DTS submission to u-boot: Downstream --> Mainline Kernel --> Mainline U-Boot So we have at least one review by the devicetree maintainers and avoid inconsistence. It would be nice to keep this. Stefan > > Regards, > Matthias
Hi Stefan, On 18 July 2019 19:09:09 BST, Stefan Wahren <wahrenst@gmx.net> wrote: >Hi Matthias, > >Am 18.07.19 um 18:48 schrieb Matthias Brugger: >> >> On 18/07/2019 18:40, Florian Fainelli wrote: >>> >>> On 7/18/2019 1:34 AM, Matthias Brugger wrote: >>> >>> [snip] >>> >>>>> static const struct of_device_id sdhci_iproc_of_match[] = { >>>>> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, >>>>> + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, >>>> As far as I'm aware of, the RPi4 FW provides a device-tree with >compatible: >>>> brcm,bcm2711-emmc2. Shouldn't we add this as well so that we can >use the DT >>>> passed by the FW? >>> Downstream typically used 2708, 2709, 2710 because those are the >>> Broadcom internal part numbers, and upstream has been using what's >on >>> the package: 2835, 2836, 2837, 2838. At the end of the day, it does >not >>> make much functional difference, but if if we have to be consistent, >>> then Stefan's approach here follows the consistency here. >>> >> So I propose to add both, so that we can use the upstream kernel with >downstream >> devcie-tree. I'm thinking of the device-tree provided at run-time by >the FW. > >sorry for the second mail, but this is a slightly different topic. I've >seen that Andrei already submitted a patch series for u-boot with a >different DTS file. > >In the past, we had the following workflow for DTS submission to >u-boot: > >Downstream --> Mainline Kernel --> Mainline U-Boot > >So we have at least one review by the devicetree maintainers and avoid >inconsistence. It would be nice to keep this. > I had a private discussion with Matthias and we dropped the dts files completely relying only on the one provided by the firmware. I'll post a v2 tomorrow. -- Andrei Gherzan gpg: rsa4096/D4D94F67AD0E9640 | t: @agherzan
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c index 2feb4ef..353cf997 100644 --- a/drivers/mmc/host/sdhci-iproc.c +++ b/drivers/mmc/host/sdhci-iproc.c @@ -261,8 +261,17 @@ static const struct sdhci_iproc_data bcm2835_data = { .mmc_caps = 0x00000000, }; +static const struct sdhci_pltfm_data sdhci_bcm2838_pltfm_data = { + .ops = &sdhci_iproc_32only_ops, +}; + +static const struct sdhci_iproc_data bcm2838_data = { + .pdata = &sdhci_bcm2838_pltfm_data, +}; + static const struct of_device_id sdhci_iproc_of_match[] = { { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data }, + { .compatible = "brcm,bcm2838-emmc2", .data = &bcm2838_data }, { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data}, { .compatible = "brcm,sdhci-iproc", .data = &iproc_data }, { }
The additional emmc2 interface of the BCM2838 is an improved version of the old emmc controller, which is able to provide DDR50 mode on the Raspberry Pi 4. Except 32 bit only register access no other quirks are known yet. Signed-off-by: Stefan Wahren <wahrenst@gmx.net> --- drivers/mmc/host/sdhci-iproc.c | 9 +++++++++ 1 file changed, 9 insertions(+) -- 2.7.4