| Message ID | 20220928121149.1724-2-rogerq@kernel.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Introduce TI GPMC memory controller driver | expand |
Hi Roger, On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > > We will need ti-gpmc driver for SPL. Allow memory drivers > do be built for SPL. > > Signed-off-by: Roger Quadros <rogerq@kernel.org> > --- > scripts/Makefile.spl | 1 + > 1 file changed, 1 insertion(+) Please can you use the existing drivers/ram directory? > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl > index 3bafeb4fe9..110076b22f 100644 > --- a/scripts/Makefile.spl > +++ b/scripts/Makefile.spl > @@ -114,6 +114,7 @@ libs-$(CONFIG_PARTITIONS) += disk/ > endif > > libs-y += drivers/ > +libs-y += drivers/memory/ > libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/dwc3/ > libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/cdns3/ > libs-y += dts/ > -- > 2.17.1 > Regards, Simon
Hi Simon, On 28/09/2022 19:27, Simon Glass wrote: > Hi Roger, > > On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: >> >> We will need ti-gpmc driver for SPL. Allow memory drivers >> do be built for SPL. >> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> scripts/Makefile.spl | 1 + >> 1 file changed, 1 insertion(+) > > Please can you use the existing drivers/ram directory? The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. It is a more general purpose controller that can support different peripherals. It is similar to the drivers already existing in the divers/memory directory. I was just trying to keep the file layout similar to that in the Linux kernel. Do you still see a problem with it? > >> >> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl >> index 3bafeb4fe9..110076b22f 100644 >> --- a/scripts/Makefile.spl >> +++ b/scripts/Makefile.spl >> @@ -114,6 +114,7 @@ libs-$(CONFIG_PARTITIONS) += disk/ >> endif >> >> libs-y += drivers/ >> +libs-y += drivers/memory/ >> libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/dwc3/ >> libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/cdns3/ >> libs-y += dts/ >> -- >> 2.17.1 >> > > Regards, > Simon cheers, -roger
On Thu, Sep 29, 2022 at 10:02:54AM +0300, Roger Quadros wrote: > Hi Simon, > > On 28/09/2022 19:27, Simon Glass wrote: > > Hi Roger, > > > > On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > >> > >> We will need ti-gpmc driver for SPL. Allow memory drivers > >> do be built for SPL. > >> > >> Signed-off-by: Roger Quadros <rogerq@kernel.org> > >> --- > >> scripts/Makefile.spl | 1 + > >> 1 file changed, 1 insertion(+) > > > > Please can you use the existing drivers/ram directory? > > The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. > It is a more general purpose controller that can support different peripherals. > It is similar to the drivers already existing in the divers/memory directory. > > I was just trying to keep the file layout similar to that in the Linux kernel. > > Do you still see a problem with it? Yes, this makes sense because it's not a ram controller driver.
Hi Roger, On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: > > Hi Simon, > > On 28/09/2022 19:27, Simon Glass wrote: > > Hi Roger, > > > > On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > >> > >> We will need ti-gpmc driver for SPL. Allow memory drivers > >> do be built for SPL. > >> > >> Signed-off-by: Roger Quadros <rogerq@kernel.org> > >> --- > >> scripts/Makefile.spl | 1 + > >> 1 file changed, 1 insertion(+) > > > > Please can you use the existing drivers/ram directory? > > The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. > It is a more general purpose controller that can support different peripherals. > It is similar to the drivers already existing in the divers/memory directory. > > I was just trying to keep the file layout similar to that in the Linux kernel. > > Do you still see a problem with it? Well in this case perhaps the RAM device would be a child of this one? But there is no uclass for your new device. One of the drivers in that dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS So let's add a uclass for it and describe exactly what it is for. Regards, Simon
Simon, On 29/09/2022 21:06, Simon Glass wrote: > Hi Roger, > > On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: >> >> Hi Simon, >> >> On 28/09/2022 19:27, Simon Glass wrote: >>> Hi Roger, >>> >>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: >>>> >>>> We will need ti-gpmc driver for SPL. Allow memory drivers >>>> do be built for SPL. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>>> --- >>>> scripts/Makefile.spl | 1 + >>>> 1 file changed, 1 insertion(+) >>> >>> Please can you use the existing drivers/ram directory? >> >> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. >> It is a more general purpose controller that can support different peripherals. >> It is similar to the drivers already existing in the divers/memory directory. >> >> I was just trying to keep the file layout similar to that in the Linux kernel. >> >> Do you still see a problem with it? > > Well in this case perhaps the RAM device would be a child of this one? That's right. > > But there is no uclass for your new device. One of the drivers in that > dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS > > So let's add a uclass for it and describe exactly what it is for. Why isn't UCLASS_SIMPLE_BUS sufficient? By itself, the GPMC driver doesn't offer any usable functionality. It just configures the bus interface and then populates the children. cheers, -roger
Hi Roger, On Fri, 30 Sept 2022 at 06:47, Roger Quadros <rogerq@kernel.org> wrote: > > Simon, > > On 29/09/2022 21:06, Simon Glass wrote: > > Hi Roger, > > > > On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: > >> > >> Hi Simon, > >> > >> On 28/09/2022 19:27, Simon Glass wrote: > >>> Hi Roger, > >>> > >>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > >>>> > >>>> We will need ti-gpmc driver for SPL. Allow memory drivers > >>>> do be built for SPL. > >>>> > >>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> > >>>> --- > >>>> scripts/Makefile.spl | 1 + > >>>> 1 file changed, 1 insertion(+) > >>> > >>> Please can you use the existing drivers/ram directory? > >> > >> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. > >> It is a more general purpose controller that can support different peripherals. > >> It is similar to the drivers already existing in the divers/memory directory. > >> > >> I was just trying to keep the file layout similar to that in the Linux kernel. > >> > >> Do you still see a problem with it? > > > > Well in this case perhaps the RAM device would be a child of this one? > > That's right. > > > > But there is no uclass for your new device. One of the drivers in that > > dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS > > > > So let's add a uclass for it and describe exactly what it is for. > > Why isn't UCLASS_SIMPLE_BUS sufficient? > By itself, the GPMC driver doesn't offer any usable functionality. > It just configures the bus interface and then populates the children. That's OK, but in that case it should go in drivers/bus and perhaps drivers/memory should go away? Regards, Simon
On Fri, Sep 30, 2022 at 07:28:51AM -0600, Simon Glass wrote: > Hi Roger, > > On Fri, 30 Sept 2022 at 06:47, Roger Quadros <rogerq@kernel.org> wrote: > > > > Simon, > > > > On 29/09/2022 21:06, Simon Glass wrote: > > > Hi Roger, > > > > > > On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: > > >> > > >> Hi Simon, > > >> > > >> On 28/09/2022 19:27, Simon Glass wrote: > > >>> Hi Roger, > > >>> > > >>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > > >>>> > > >>>> We will need ti-gpmc driver for SPL. Allow memory drivers > > >>>> do be built for SPL. > > >>>> > > >>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> > > >>>> --- > > >>>> scripts/Makefile.spl | 1 + > > >>>> 1 file changed, 1 insertion(+) > > >>> > > >>> Please can you use the existing drivers/ram directory? > > >> > > >> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. > > >> It is a more general purpose controller that can support different peripherals. > > >> It is similar to the drivers already existing in the divers/memory directory. > > >> > > >> I was just trying to keep the file layout similar to that in the Linux kernel. > > >> > > >> Do you still see a problem with it? > > > > > > Well in this case perhaps the RAM device would be a child of this one? > > > > That's right. > > > > > > But there is no uclass for your new device. One of the drivers in that > > > dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS > > > > > > So let's add a uclass for it and describe exactly what it is for. > > > > Why isn't UCLASS_SIMPLE_BUS sufficient? > > By itself, the GPMC driver doesn't offer any usable functionality. > > It just configures the bus interface and then populates the children. > > That's OK, but in that case it should go in drivers/bus and perhaps > drivers/memory should go away? No, drivers/memory/stm32-fmc2-ebi.c is there and is the equivalent (more or less) of drivers/memory/stm32-fmc2-ebi.c in the linux kernel. So maybe a question is, are we talking about the equivalent of drivers/memory/omap-gpmc.c here? Or something else?
On 30/09/2022 17:00, Tom Rini wrote: > On Fri, Sep 30, 2022 at 07:28:51AM -0600, Simon Glass wrote: >> Hi Roger, >> >> On Fri, 30 Sept 2022 at 06:47, Roger Quadros <rogerq@kernel.org> wrote: >>> >>> Simon, >>> >>> On 29/09/2022 21:06, Simon Glass wrote: >>>> Hi Roger, >>>> >>>> On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: >>>>> >>>>> Hi Simon, >>>>> >>>>> On 28/09/2022 19:27, Simon Glass wrote: >>>>>> Hi Roger, >>>>>> >>>>>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: >>>>>>> >>>>>>> We will need ti-gpmc driver for SPL. Allow memory drivers >>>>>>> do be built for SPL. >>>>>>> >>>>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>>>>>> --- >>>>>>> scripts/Makefile.spl | 1 + >>>>>>> 1 file changed, 1 insertion(+) >>>>>> >>>>>> Please can you use the existing drivers/ram directory? >>>>> >>>>> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. >>>>> It is a more general purpose controller that can support different peripherals. >>>>> It is similar to the drivers already existing in the divers/memory directory. >>>>> >>>>> I was just trying to keep the file layout similar to that in the Linux kernel. >>>>> >>>>> Do you still see a problem with it? >>>> >>>> Well in this case perhaps the RAM device would be a child of this one? >>> >>> That's right. >>>> >>>> But there is no uclass for your new device. One of the drivers in that >>>> dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS >>>> >>>> So let's add a uclass for it and describe exactly what it is for. >>> >>> Why isn't UCLASS_SIMPLE_BUS sufficient? >>> By itself, the GPMC driver doesn't offer any usable functionality. >>> It just configures the bus interface and then populates the children. >> >> That's OK, but in that case it should go in drivers/bus and perhaps >> drivers/memory should go away? > > No, drivers/memory/stm32-fmc2-ebi.c is there and is the equivalent (more > or less) of drivers/memory/stm32-fmc2-ebi.c in the linux kernel. So > maybe a question is, are we talking about the equivalent of > drivers/memory/omap-gpmc.c here? Or something else? > Yes, I just picked up that file and adapted it for u-boot. As GPMC is not only for OMAP devices anymore I renamed it to ti-gpmc.c. If it helps we can retain the kernel naming. cheers, -roger
Hi, On Fri, 30 Sept 2022 at 14:18, Roger Quadros <rogerq@kernel.org> wrote: > > On 30/09/2022 17:00, Tom Rini wrote: > > On Fri, Sep 30, 2022 at 07:28:51AM -0600, Simon Glass wrote: > >> Hi Roger, > >> > >> On Fri, 30 Sept 2022 at 06:47, Roger Quadros <rogerq@kernel.org> wrote: > >>> > >>> Simon, > >>> > >>> On 29/09/2022 21:06, Simon Glass wrote: > >>>> Hi Roger, > >>>> > >>>> On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: > >>>>> > >>>>> Hi Simon, > >>>>> > >>>>> On 28/09/2022 19:27, Simon Glass wrote: > >>>>>> Hi Roger, > >>>>>> > >>>>>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > >>>>>>> > >>>>>>> We will need ti-gpmc driver for SPL. Allow memory drivers > >>>>>>> do be built for SPL. > >>>>>>> > >>>>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> > >>>>>>> --- > >>>>>>> scripts/Makefile.spl | 1 + > >>>>>>> 1 file changed, 1 insertion(+) > >>>>>> > >>>>>> Please can you use the existing drivers/ram directory? > >>>>> > >>>>> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. > >>>>> It is a more general purpose controller that can support different peripherals. > >>>>> It is similar to the drivers already existing in the divers/memory directory. > >>>>> > >>>>> I was just trying to keep the file layout similar to that in the Linux kernel. > >>>>> > >>>>> Do you still see a problem with it? > >>>> > >>>> Well in this case perhaps the RAM device would be a child of this one? > >>> > >>> That's right. > >>>> > >>>> But there is no uclass for your new device. One of the drivers in that > >>>> dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS > >>>> > >>>> So let's add a uclass for it and describe exactly what it is for. > >>> > >>> Why isn't UCLASS_SIMPLE_BUS sufficient? > >>> By itself, the GPMC driver doesn't offer any usable functionality. > >>> It just configures the bus interface and then populates the children. > >> > >> That's OK, but in that case it should go in drivers/bus and perhaps > >> drivers/memory should go away? > > > > No, drivers/memory/stm32-fmc2-ebi.c is there and is the equivalent (more > > or less) of drivers/memory/stm32-fmc2-ebi.c in the linux kernel. So > > maybe a question is, are we talking about the equivalent of > > drivers/memory/omap-gpmc.c here? Or something else? > > > > Yes, I just picked up that file and adapted it for u-boot. > As GPMC is not only for OMAP devices anymore I renamed it to ti-gpmc.c. > If it helps we can retain the kernel naming. In the fullness of time all driver/xxx directories should have a uclass associated with them. So in this case, do we want to use UCLASS_BUS and put it in drivers/bus or add a UCLASS_MEMORY and put it in drivers/memory? Regards, Simon
Hi, On 01/10/2022 02:48, Simon Glass wrote: > Hi, > > On Fri, 30 Sept 2022 at 14:18, Roger Quadros <rogerq@kernel.org> wrote: >> >> On 30/09/2022 17:00, Tom Rini wrote: >>> On Fri, Sep 30, 2022 at 07:28:51AM -0600, Simon Glass wrote: >>>> Hi Roger, >>>> >>>> On Fri, 30 Sept 2022 at 06:47, Roger Quadros <rogerq@kernel.org> wrote: >>>>> >>>>> Simon, >>>>> >>>>> On 29/09/2022 21:06, Simon Glass wrote: >>>>>> Hi Roger, >>>>>> >>>>>> On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: >>>>>>> >>>>>>> Hi Simon, >>>>>>> >>>>>>> On 28/09/2022 19:27, Simon Glass wrote: >>>>>>>> Hi Roger, >>>>>>>> >>>>>>>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: >>>>>>>>> >>>>>>>>> We will need ti-gpmc driver for SPL. Allow memory drivers >>>>>>>>> do be built for SPL. >>>>>>>>> >>>>>>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>>>>>>>> --- >>>>>>>>> scripts/Makefile.spl | 1 + >>>>>>>>> 1 file changed, 1 insertion(+) >>>>>>>> >>>>>>>> Please can you use the existing drivers/ram directory? >>>>>>> >>>>>>> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. >>>>>>> It is a more general purpose controller that can support different peripherals. >>>>>>> It is similar to the drivers already existing in the divers/memory directory. >>>>>>> >>>>>>> I was just trying to keep the file layout similar to that in the Linux kernel. >>>>>>> >>>>>>> Do you still see a problem with it? >>>>>> >>>>>> Well in this case perhaps the RAM device would be a child of this one? >>>>> >>>>> That's right. >>>>>> >>>>>> But there is no uclass for your new device. One of the drivers in that >>>>>> dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS >>>>>> >>>>>> So let's add a uclass for it and describe exactly what it is for. >>>>> >>>>> Why isn't UCLASS_SIMPLE_BUS sufficient? >>>>> By itself, the GPMC driver doesn't offer any usable functionality. >>>>> It just configures the bus interface and then populates the children. >>>> >>>> That's OK, but in that case it should go in drivers/bus and perhaps >>>> drivers/memory should go away? >>> >>> No, drivers/memory/stm32-fmc2-ebi.c is there and is the equivalent (more >>> or less) of drivers/memory/stm32-fmc2-ebi.c in the linux kernel. So >>> maybe a question is, are we talking about the equivalent of >>> drivers/memory/omap-gpmc.c here? Or something else? >>> >> >> Yes, I just picked up that file and adapted it for u-boot. >> As GPMC is not only for OMAP devices anymore I renamed it to ti-gpmc.c. >> If it helps we can retain the kernel naming. > > In the fullness of time all driver/xxx directories should have a > uclass associated with them. So in this case, do we want to use > UCLASS_BUS and put it in drivers/bus or add a UCLASS_MEMORY and put it > in drivers/memory? I'd opt for UCLASS_MEMORY. cheers, -roger
Hi Roger, On Sat, 1 Oct 2022 at 02:37, Roger Quadros <rogerq@kernel.org> wrote: > > Hi, > > On 01/10/2022 02:48, Simon Glass wrote: > > Hi, > > > > On Fri, 30 Sept 2022 at 14:18, Roger Quadros <rogerq@kernel.org> wrote: > >> > >> On 30/09/2022 17:00, Tom Rini wrote: > >>> On Fri, Sep 30, 2022 at 07:28:51AM -0600, Simon Glass wrote: > >>>> Hi Roger, > >>>> > >>>> On Fri, 30 Sept 2022 at 06:47, Roger Quadros <rogerq@kernel.org> wrote: > >>>>> > >>>>> Simon, > >>>>> > >>>>> On 29/09/2022 21:06, Simon Glass wrote: > >>>>>> Hi Roger, > >>>>>> > >>>>>> On Thu, 29 Sept 2022 at 01:03, Roger Quadros <rogerq@kernel.org> wrote: > >>>>>>> > >>>>>>> Hi Simon, > >>>>>>> > >>>>>>> On 28/09/2022 19:27, Simon Glass wrote: > >>>>>>>> Hi Roger, > >>>>>>>> > >>>>>>>> On Wed, 28 Sept 2022 at 06:12, Roger Quadros <rogerq@kernel.org> wrote: > >>>>>>>>> > >>>>>>>>> We will need ti-gpmc driver for SPL. Allow memory drivers > >>>>>>>>> do be built for SPL. > >>>>>>>>> > >>>>>>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org> > >>>>>>>>> --- > >>>>>>>>> scripts/Makefile.spl | 1 + > >>>>>>>>> 1 file changed, 1 insertion(+) > >>>>>>>> > >>>>>>>> Please can you use the existing drivers/ram directory? > >>>>>>> > >>>>>>> The ti-gpmc driver is not actually a RAM only controller. Although it can support SRAM. > >>>>>>> It is a more general purpose controller that can support different peripherals. > >>>>>>> It is similar to the drivers already existing in the divers/memory directory. > >>>>>>> > >>>>>>> I was just trying to keep the file layout similar to that in the Linux kernel. > >>>>>>> > >>>>>>> Do you still see a problem with it? > >>>>>> > >>>>>> Well in this case perhaps the RAM device would be a child of this one? > >>>>> > >>>>> That's right. > >>>>>> > >>>>>> But there is no uclass for your new device. One of the drivers in that > >>>>>> dir uses UCLASS_NOP and your one seems to use UCLASS_SIMPLE_BUS > >>>>>> > >>>>>> So let's add a uclass for it and describe exactly what it is for. > >>>>> > >>>>> Why isn't UCLASS_SIMPLE_BUS sufficient? > >>>>> By itself, the GPMC driver doesn't offer any usable functionality. > >>>>> It just configures the bus interface and then populates the children. > >>>> > >>>> That's OK, but in that case it should go in drivers/bus and perhaps > >>>> drivers/memory should go away? > >>> > >>> No, drivers/memory/stm32-fmc2-ebi.c is there and is the equivalent (more > >>> or less) of drivers/memory/stm32-fmc2-ebi.c in the linux kernel. So > >>> maybe a question is, are we talking about the equivalent of > >>> drivers/memory/omap-gpmc.c here? Or something else? > >>> > >> > >> Yes, I just picked up that file and adapted it for u-boot. > >> As GPMC is not only for OMAP devices anymore I renamed it to ti-gpmc.c. > >> If it helps we can retain the kernel naming. > > > > In the fullness of time all driver/xxx directories should have a > > uclass associated with them. So in this case, do we want to use > > UCLASS_BUS and put it in drivers/bus or add a UCLASS_MEMORY and put it > > in drivers/memory? > > I'd opt for UCLASS_MEMORY. OK, here is an example patch for adding a new uclass: https://patchwork.ozlabs.org/project/uboot/patch/20220930120430.42307-2-post@lespocky.de/ It is the bare minimum as it has not methods, but it seems like yours won't either. Regards, Simon
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 3bafeb4fe9..110076b22f 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -114,6 +114,7 @@ libs-$(CONFIG_PARTITIONS) += disk/ endif libs-y += drivers/ +libs-y += drivers/memory/ libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/dwc3/ libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/cdns3/ libs-y += dts/
We will need ti-gpmc driver for SPL. Allow memory drivers do be built for SPL. Signed-off-by: Roger Quadros <rogerq@kernel.org> --- scripts/Makefile.spl | 1 + 1 file changed, 1 insertion(+)