| Message ID | 20210108211339.1724769-1-jeremy.linton@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci-iproc: Add ACPI bindings for the rpi4 | expand |
On 1/8/2021 1:13 PM, Jeremy Linton wrote: > The rpi4 has a Arasan controller it carries over > from the rpi3, and a newer eMMC2 controller. > Because of a couple "quirks" it seems wiser to bind > these controllers to the same driver that DT is using > on this platform rather than the generic sdhci_acpi > driver with PNP0D40. > > So, we use BCM2847 for the older Arasan and BRCME88C > for the newer eMMC2. > > With this change linux is capable of utilizing the > SD card slot, and the wifi on this platform > with linux. > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> As far as ACPI ID's go: Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Hi Jeremy,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.11-rc2 next-20210108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jeremy-Linton/mmc-sdhci-iproc-Add-ACPI-bindings-for-the-rpi4/20210109-051645
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6279d812eab67a6df6b22fa495201db6f2305924
config: riscv-randconfig-r012-20210108 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bc556e5685c0f97e79fb7b3c6f15cc5062db8e36)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/659eacf5a5de971ea94390dd6c7443c82d53ea5e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jeremy-Linton/mmc-sdhci-iproc-Add-ACPI-bindings-for-the-rpi4/20210109-051645
git checkout 659eacf5a5de971ea94390dd6c7443c82d53ea5e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:556:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inb(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:55:76: note: expanded from macro 'inb'
#define inb(c) ({ u8 __v; __io_pbr(); __v = readb_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:87:48: note: expanded from macro 'readb_cpu'
#define readb_cpu(c) ({ u8 __r = __raw_readb(c); __r; })
^
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:564:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inw(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:56:76: note: expanded from macro 'inw'
#define inw(c) ({ u16 __v; __io_pbr(); __v = readw_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:88:76: note: expanded from macro 'readw_cpu'
#define readw_cpu(c) ({ u16 __r = le16_to_cpu((__force __le16)__raw_readw(c)); __r; })
^
include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:572:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inl(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:57:76: note: expanded from macro 'inl'
#define inl(c) ({ u32 __v; __io_pbr(); __v = readl_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:89:76: note: expanded from macro 'readl_cpu'
#define readl_cpu(c) ({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; })
^
include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:580:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outb(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:59:68: note: expanded from macro 'outb'
#define outb(v,c) ({ __io_pbw(); writeb_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:91:52: note: expanded from macro 'writeb_cpu'
#define writeb_cpu(v, c) ((void)__raw_writeb((v), (c)))
^
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:588:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outw(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:60:68: note: expanded from macro 'outw'
#define outw(v,c) ({ __io_pbw(); writew_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:92:76: note: expanded from macro 'writew_cpu'
#define writew_cpu(v, c) ((void)__raw_writew((__force u16)cpu_to_le16(v), (c)))
^
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:596:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outl(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:61:68: note: expanded from macro 'outl'
#define outl(v,c) ({ __io_pbw(); writel_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:93:76: note: expanded from macro 'writel_cpu'
#define writel_cpu(v, c) ((void)__raw_writel((__force u32)cpu_to_le32(v), (c)))
^
In file included from drivers/mmc/host/sdhci-iproc.c:24:
In file included from drivers/mmc/host/sdhci-pltfm.h:13:
In file included from drivers/mmc/host/sdhci.h:13:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:1005:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> drivers/mmc/host/sdhci-iproc.c:272:38: warning: unused variable 'bcm_arasan_data' [-Wunused-const-variable]
static const struct sdhci_iproc_data bcm_arasan_data = {
^
8 warnings generated.
vim +/bcm_arasan_data +272 drivers/mmc/host/sdhci-iproc.c
271
> 272 static const struct sdhci_iproc_data bcm_arasan_data = {
273 .pdata = &sdhci_bcm_arasan_data,
274 };
275
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi, On 1/8/21 7:10 PM, kernel test robot wrote: > Hi Jeremy, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on linus/master] > [also build test WARNING on v5.11-rc2 next-20210108] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Jeremy-Linton/mmc-sdhci-iproc-Add-ACPI-bindings-for-the-rpi4/20210109-051645 > base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6279d812eab67a6df6b22fa495201db6f2305924 > config: riscv-randconfig-r012-20210108 (attached as .config) > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bc556e5685c0f97e79fb7b3c6f15cc5062db8e36) > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # install riscv cross compiling tool for clang build > # apt-get install binutils-riscv64-linux-gnu > # https://github.com/0day-ci/linux/commit/659eacf5a5de971ea94390dd6c7443c82d53ea5e > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Jeremy-Linton/mmc-sdhci-iproc-Add-ACPI-bindings-for-the-rpi4/20210109-051645 > git checkout 659eacf5a5de971ea94390dd6c7443c82d53ea5e > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): (trimming) > include/asm-generic/io.h:1005:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port; > ~~~~~~~~~~ ^ >>> drivers/mmc/host/sdhci-iproc.c:272:38: warning: unused variable 'bcm_arasan_data' [-Wunused-const-variable] > static const struct sdhci_iproc_data bcm_arasan_data = { I think this is the only one caused by this patch, and its because the new structures are only used inside the #ifdef ACPI block. I will post a v2. > ^ > 8 warnings generated. > > > vim +/bcm_arasan_data +272 drivers/mmc/host/sdhci-iproc.c > > 271 > > 272 static const struct sdhci_iproc_data bcm_arasan_data = { > 273 .pdata = &sdhci_bcm_arasan_data, > 274 }; > 275 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org >
Hi Jeremy, +add Nicolas Am 08.01.21 um 22:13 schrieb Jeremy Linton: > The rpi4 has a Arasan controller it carries over > from the rpi3, and a newer eMMC2 controller. > Because of a couple "quirks" it seems wiser to bind > these controllers to the same driver that DT is using > on this platform rather than the generic sdhci_acpi > driver with PNP0D40. > > So, we use BCM2847 for the older Arasan and BRCME88C > for the newer eMMC2. > > With this change linux is capable of utilizing the > SD card slot, and the wifi on this platform > with linux. > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > --- > drivers/mmc/host/sdhci-iproc.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index c9434b461aab..f79d97b41805 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = { > .ops = &sdhci_iproc_32only_ops, > }; > > +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = { > + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | > + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | > + SDHCI_QUIRK_NO_HISPD_BIT, > + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, > + .ops = &sdhci_iproc_32only_ops, > +}; Why do we need an almost exact copy of bcm2835_data which works fine for all Raspberry Pi boards? > + > static const struct sdhci_iproc_data bcm2835_data = { > .pdata = &sdhci_bcm2835_pltfm_data, > .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT) > @@ -261,6 +269,10 @@ static const struct sdhci_iproc_data bcm2835_data = { > .mmc_caps = 0x00000000, > }; > > +static const struct sdhci_iproc_data bcm_arasan_data = { > + .pdata = &sdhci_bcm_arasan_data, > +}; > + > static const struct sdhci_ops sdhci_iproc_bcm2711_ops = { > .read_l = sdhci_iproc_readl, > .read_w = sdhci_iproc_readw, > @@ -299,6 +311,8 @@ MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match); > static const struct acpi_device_id sdhci_iproc_acpi_ids[] = { > { .id = "BRCM5871", .driver_data = (kernel_ulong_t)&iproc_cygnus_data }, > { .id = "BRCM5872", .driver_data = (kernel_ulong_t)&iproc_data }, > + { .id = "BCM2847", .driver_data = (kernel_ulong_t)&bcm_arasan_data }, Sorry, i don't have deeper knowledge about ACPI, but BCM2837 is the official naming of the SoC on the RPi 3. Is this a typo in the id? > + { .id = "BRCME88C", .driver_data = (kernel_ulong_t)&bcm2711_data }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(acpi, sdhci_iproc_acpi_ids);
Hi, On 1/9/21 5:07 AM, Stefan Wahren wrote: > Hi Jeremy, > > +add Nicolas > > Am 08.01.21 um 22:13 schrieb Jeremy Linton: >> The rpi4 has a Arasan controller it carries over >> from the rpi3, and a newer eMMC2 controller. >> Because of a couple "quirks" it seems wiser to bind >> these controllers to the same driver that DT is using >> on this platform rather than the generic sdhci_acpi >> driver with PNP0D40. >> >> So, we use BCM2847 for the older Arasan and BRCME88C >> for the newer eMMC2. >> >> With this change linux is capable of utilizing the >> SD card slot, and the wifi on this platform >> with linux. >> >> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> >> --- >> drivers/mmc/host/sdhci-iproc.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c >> index c9434b461aab..f79d97b41805 100644 >> --- a/drivers/mmc/host/sdhci-iproc.c >> +++ b/drivers/mmc/host/sdhci-iproc.c >> @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = { >> .ops = &sdhci_iproc_32only_ops, >> }; >> >> +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = { >> + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | >> + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | >> + SDHCI_QUIRK_NO_HISPD_BIT, >> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, >> + .ops = &sdhci_iproc_32only_ops, >> +}; First, thanks for taking a look at this! > Why do we need an almost exact copy of bcm2835_data which works fine for > all Raspberry Pi boards? The short answer to the remainder of this email is that i'm trying to continue supporting existing OSs (windows) using the ACPI tables on the rpi3/rpi4 while adding rpi4+Linux support. An even shorter answer is they don't work because ACPI doesn't provide the same clock/attributes/etc controls that exist with DT. So, what happened here is that I got this controller "working" with the generic PNP0D40 sdhci_acpi driver. I managed this only by controlling the sdhci_caps/masks in the firmware. In theory this minimizes the amount of work needed on the other OS which are booting on the same ACPI tables (*bsds). They should only need to quirk the bcm/arasan specific functionality, rather than some of the quirking which change the caps behavior. But because we don't know which if any of the older rpi/arasan quirks are still needed the safest solution is to use the _iproc driver and just drop the quirk flags known to be worked around by the firmware caps override. >> + >> static const struct sdhci_iproc_data bcm2835_data = { >> .pdata = &sdhci_bcm2835_pltfm_data, >> .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT) >> @@ -261,6 +269,10 @@ static const struct sdhci_iproc_data bcm2835_data = { >> .mmc_caps = 0x00000000, >> }; >> >> +static const struct sdhci_iproc_data bcm_arasan_data = { >> + .pdata = &sdhci_bcm_arasan_data, >> +}; >> + >> static const struct sdhci_ops sdhci_iproc_bcm2711_ops = { >> .read_l = sdhci_iproc_readl, >> .read_w = sdhci_iproc_readw, >> @@ -299,6 +311,8 @@ MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match); >> static const struct acpi_device_id sdhci_iproc_acpi_ids[] = { >> { .id = "BRCM5871", .driver_data = (kernel_ulong_t)&iproc_cygnus_data }, >> { .id = "BRCM5872", .driver_data = (kernel_ulong_t)&iproc_data }, >> + { .id = "BCM2847", .driver_data = (kernel_ulong_t)&bcm_arasan_data }, > > Sorry, i don't have deeper knowledge about ACPI, but BCM2837 is the > official naming of the SoC on the RPi 3. > > Is this a typo in the id? Not really. Some background: The PFTF is basically the custodian of the combined rpi3 port done by Microsoft and a few other peoples/organizations ports. That merged code base was upstreamed a couple years ago to edk2 for the rpi3 and is the official port. On the rpi3+uefi platform, linux is just using DT, but windows and possibly other OSs are using the ACPI tables. For the Rpi4, the intentions is to be an ACPI first platform, but we are inheriting the rpi3 legacy peripheral descriptions. So, for the past year+ everyone has been basing their rpi4 ACPI OS ports on those tables and only adjusting them in backwards compatible ways. Meaning, that a few years back someone put that ID in the rpi3 ACPI tables, and now we are stuck with it unless we are willing to break other OSs. > >> + { .id = "BRCME88C", .driver_data = (kernel_ulong_t)&bcm2711_data }, >> { /* sentinel */ } >> }; >> MODULE_DEVICE_TABLE(acpi, sdhci_iproc_acpi_ids); >
On Mon, 11 Jan 2021 at 04:40, Jeremy Linton <jeremy.linton@arm.com> wrote: > > Hi, > > On 1/9/21 5:07 AM, Stefan Wahren wrote: > > Hi Jeremy, > > > > +add Nicolas > > > > Am 08.01.21 um 22:13 schrieb Jeremy Linton: > >> The rpi4 has a Arasan controller it carries over ... > >> @@ -299,6 +311,8 @@ MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match); > >> static const struct acpi_device_id sdhci_iproc_acpi_ids[] = { > >> { .id = "BRCM5871", .driver_data = (kernel_ulong_t)&iproc_cygnus_data }, > >> { .id = "BRCM5872", .driver_data = (kernel_ulong_t)&iproc_data }, > >> + { .id = "BCM2847", .driver_data = (kernel_ulong_t)&bcm_arasan_data }, > > > > Sorry, i don't have deeper knowledge about ACPI, but BCM2837 is the > > official naming of the SoC on the RPi 3. > > > > Is this a typo in the id? > > Not really. > > Some background: The PFTF is basically the custodian of the combined > rpi3 port done by Microsoft and a few other peoples/organizations ports. > That merged code base was upstreamed a couple years ago to edk2 for the > rpi3 and is the official port. On the rpi3+uefi platform, linux is just > using DT, but windows and possibly other OSs are using the ACPI tables. > For the Rpi4, the intentions is to be an ACPI first platform, but we are > inheriting the rpi3 legacy peripheral descriptions. I wouldn't say ACPI first - Linux will likely always have far better DT coverage for these platforms, with DT overlays etc. However, there is a strong pull from the industry to support Windows, VMware, RHEL/Centos and the BSDs on these systems, which is why the ACPI firmware port is important. RPi4 is also the most easily obtained Linux/arm64 machine with a proper and fairly complete implementation of standards-based rich firmware, which is why it makes sense to support both ACPI and DT boot on it in Linux. > So, for the past > year+ everyone has been basing their rpi4 ACPI OS ports on those tables > and only adjusting them in backwards compatible ways. > > Meaning, that a few years back someone put that ID in the rpi3 ACPI > tables, and now we are stuck with it unless we are willing to break > other OSs. > Note that most of the ACPI tables were contributed by Microsoft in order to boot Windows for IOT (or whatever it was called at the time) on the RPi3; they weren't just pulled out of thin air. > > > > >> + { .id = "BRCME88C", .driver_data = (kernel_ulong_t)&bcm2711_data }, > >> { /* sentinel */ } > >> }; > >> MODULE_DEVICE_TABLE(acpi, sdhci_iproc_acpi_ids); > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, Am 11.01.21 um 04:39 schrieb Jeremy Linton: > Hi, > > On 1/9/21 5:07 AM, Stefan Wahren wrote: >> Hi Jeremy, >> >> +add Nicolas >> >> Am 08.01.21 um 22:13 schrieb Jeremy Linton: >>> The rpi4 has a Arasan controller it carries over >>> from the rpi3, and a newer eMMC2 controller. >>> Because of a couple "quirks" it seems wiser to bind >>> these controllers to the same driver that DT is using >>> on this platform rather than the generic sdhci_acpi >>> driver with PNP0D40. >>> >>> So, we use BCM2847 for the older Arasan and BRCME88C >>> for the newer eMMC2. >>> >>> With this change linux is capable of utilizing the >>> SD card slot, and the wifi on this platform >>> with linux. >>> >>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> >>> --- >>> drivers/mmc/host/sdhci-iproc.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci-iproc.c >>> b/drivers/mmc/host/sdhci-iproc.c >>> index c9434b461aab..f79d97b41805 100644 >>> --- a/drivers/mmc/host/sdhci-iproc.c >>> +++ b/drivers/mmc/host/sdhci-iproc.c >>> @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data >>> sdhci_bcm2835_pltfm_data = { >>> .ops = &sdhci_iproc_32only_ops, >>> }; >>> +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = { >>> + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | >>> + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | >>> + SDHCI_QUIRK_NO_HISPD_BIT, >>> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, >>> + .ops = &sdhci_iproc_32only_ops, >>> +}; > > First, thanks for taking a look at this! > > >> Why do we need an almost exact copy of bcm2835_data which works fine for >> all Raspberry Pi boards? > > The short answer to the remainder of this email is that i'm trying to > continue supporting existing OSs (windows) using the ACPI tables on > the rpi3/rpi4 while adding rpi4+Linux support. > > An even shorter answer is they don't work because ACPI doesn't provide > the same clock/attributes/etc controls that exist with DT. > > So, what happened here is that I got this controller "working" with > the generic PNP0D40 sdhci_acpi driver. I managed this only by > controlling the sdhci_caps/masks in the firmware. In theory this > minimizes the amount of work needed on the other OS which are booting > on the same ACPI tables (*bsds). They should only need to quirk the > bcm/arasan specific functionality, rather than some of the quirking > which change the caps behavior. But because we don't know which if any > of the older rpi/arasan quirks are still needed the safest solution is > to use the _iproc driver and just drop the quirk flags known to be > worked around by the firmware caps override. okay, thanks for the explanation. I was also confused by bcm_arasan, because there is already an Arasan specific sdhci driver. But now it's clear to me. Could you please add a short comment (above sdhci_bcm_arasan_data) why we cannot use bcm2835_data? Btw the subject isn't complete. The patch is also related to the rpi3. Best regards Stefan
Hi, On 1/11/21 6:23 AM, Stefan Wahren wrote: > Hi, > > Am 11.01.21 um 04:39 schrieb Jeremy Linton: >> Hi, >> >> On 1/9/21 5:07 AM, Stefan Wahren wrote: >>> Hi Jeremy, >>> >>> +add Nicolas >>> >>> Am 08.01.21 um 22:13 schrieb Jeremy Linton: >>>> The rpi4 has a Arasan controller it carries over >>>> from the rpi3, and a newer eMMC2 controller. >>>> Because of a couple "quirks" it seems wiser to bind >>>> these controllers to the same driver that DT is using >>>> on this platform rather than the generic sdhci_acpi >>>> driver with PNP0D40. >>>> >>>> So, we use BCM2847 for the older Arasan and BRCME88C >>>> for the newer eMMC2. >>>> >>>> With this change linux is capable of utilizing the >>>> SD card slot, and the wifi on this platform >>>> with linux. >>>> >>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> >>>> --- >>>> drivers/mmc/host/sdhci-iproc.c | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/drivers/mmc/host/sdhci-iproc.c >>>> b/drivers/mmc/host/sdhci-iproc.c >>>> index c9434b461aab..f79d97b41805 100644 >>>> --- a/drivers/mmc/host/sdhci-iproc.c >>>> +++ b/drivers/mmc/host/sdhci-iproc.c >>>> @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data >>>> sdhci_bcm2835_pltfm_data = { >>>> .ops = &sdhci_iproc_32only_ops, >>>> }; >>>> +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = { >>>> + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | >>>> + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | >>>> + SDHCI_QUIRK_NO_HISPD_BIT, >>>> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, >>>> + .ops = &sdhci_iproc_32only_ops, >>>> +}; >> >> First, thanks for taking a look at this! >> >> >>> Why do we need an almost exact copy of bcm2835_data which works fine for >>> all Raspberry Pi boards? >> >> The short answer to the remainder of this email is that i'm trying to >> continue supporting existing OSs (windows) using the ACPI tables on >> the rpi3/rpi4 while adding rpi4+Linux support. >> >> An even shorter answer is they don't work because ACPI doesn't provide >> the same clock/attributes/etc controls that exist with DT. >> >> So, what happened here is that I got this controller "working" with >> the generic PNP0D40 sdhci_acpi driver. I managed this only by >> controlling the sdhci_caps/masks in the firmware. In theory this >> minimizes the amount of work needed on the other OS which are booting >> on the same ACPI tables (*bsds). They should only need to quirk the >> bcm/arasan specific functionality, rather than some of the quirking >> which change the caps behavior. But because we don't know which if any >> of the older rpi/arasan quirks are still needed the safest solution is >> to use the _iproc driver and just drop the quirk flags known to be >> worked around by the firmware caps override. > > okay, thanks for the explanation. I was also confused by bcm_arasan, > because there is already an Arasan specific sdhci driver. But now it's > clear to me. > > Could you please add a short comment (above sdhci_bcm_arasan_data) why > we cannot use bcm2835_data? Sure. > > Btw the subject isn't complete. The patch is also related to the rpi3. Only via the historical ACPI tables. There isn't any attempt to get ACPI+Linux working on the rpi3. Its to far away from BSA. For starters it doesn't have a GIC. So while one could bind this driver on the rpi3, that would require being able to boot Linux on the rpi3 in ACPI mode. Thanks again! > > Best regards > Stefan > >
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c index c9434b461aab..f79d97b41805 100644 --- a/drivers/mmc/host/sdhci-iproc.c +++ b/drivers/mmc/host/sdhci-iproc.c @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = { .ops = &sdhci_iproc_32only_ops, }; +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = { + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | + SDHCI_QUIRK_NO_HISPD_BIT, + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, + .ops = &sdhci_iproc_32only_ops, +}; + static const struct sdhci_iproc_data bcm2835_data = { .pdata = &sdhci_bcm2835_pltfm_data, .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT) @@ -261,6 +269,10 @@ static const struct sdhci_iproc_data bcm2835_data = { .mmc_caps = 0x00000000, }; +static const struct sdhci_iproc_data bcm_arasan_data = { + .pdata = &sdhci_bcm_arasan_data, +}; + static const struct sdhci_ops sdhci_iproc_bcm2711_ops = { .read_l = sdhci_iproc_readl, .read_w = sdhci_iproc_readw, @@ -299,6 +311,8 @@ MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match); static const struct acpi_device_id sdhci_iproc_acpi_ids[] = { { .id = "BRCM5871", .driver_data = (kernel_ulong_t)&iproc_cygnus_data }, { .id = "BRCM5872", .driver_data = (kernel_ulong_t)&iproc_data }, + { .id = "BCM2847", .driver_data = (kernel_ulong_t)&bcm_arasan_data }, + { .id = "BRCME88C", .driver_data = (kernel_ulong_t)&bcm2711_data }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(acpi, sdhci_iproc_acpi_ids);
The rpi4 has a Arasan controller it carries over from the rpi3, and a newer eMMC2 controller. Because of a couple "quirks" it seems wiser to bind these controllers to the same driver that DT is using on this platform rather than the generic sdhci_acpi driver with PNP0D40. So, we use BCM2847 for the older Arasan and BRCME88C for the newer eMMC2. With this change linux is capable of utilizing the SD card slot, and the wifi on this platform with linux. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> --- drivers/mmc/host/sdhci-iproc.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)