[U-Boot] [PATCH 1/2] gpio: Add DW APB GPIO driver

[Marek Vasut]

On Wednesday, August 05, 2015 at 04:39:33 PM, Simon Glass wrote:
> Hi Marek,

Hi Simon,

> On 2 August 2015 at 18:16, Marek Vasut <marex at denx.de> wrote:
> > On Monday, August 03, 2015 at 01:38:28 AM, Simon Glass wrote:
> >> Hi Marek,
> > 
> > Hi Simon,
> > 
> > [...]
> > 
> >> >> > +               if (!fdtdec_get_bool(blob, node,
> >> >> > "gpio-controller")) +                       continue;
> >> >> > +
> >> >> > +               plat = NULL;
> >> >> > +               plat = calloc(1, sizeof(*plat));
> >> >> 
> >> >> I suppose this should use devm_alloc() now.
> >> > 
> >> > Is that even in u-boot/master ? Also, I'm not sure it's a good idea to
> >> > put this in if I use this driver in SPL.
> >> 
> >> Yes it's very new. Only in dm/master.
> >> 
> >> It's only compiled in when enabled, so you can still use it in SPL.
> >> 
> >> Up to you. At some point I think we should convert all driver allocs
> >> to use devm so that we know what allocation is going on.
> > 
> > When is this landing in master ? I don't mind either way, but I can do
> > a linting pass on the socfpga when things settle down too.
> 
> Hopefully I'll have a pull request out on Friday.

Cool!

> >> >> > +               if (!plat)
> >> >> > +                       return -ENOMEM;
> >> >> > +
> >> >> > +               plat->base = base;
> >> >> > +               plat->bank = bank;
> >> >> > +               plat->pins = fdtdec_get_int(blob, node,
> >> >> > "snps,nr-gpios", 0); +               snprintf(plat->name,
> >> >> > sizeof(plat->name) - 1, "%s-bank%i-", +                       
> >> >> > name, bank);
> >> >> 
> >> >> Why such a long name? That's going to be a pain to type in the 'gpio'
> >> >> command.
> >> > 
> >> > Do you have a suggestion please ?
> >> 
> >> A, B, C is good if you have <=26 banks. Tegra does that.
> >> 
> >> Exynos does PA0, PA1, PB0, PC0, PC1, etc.
> > 
> > How do I identify which one is PA0/PA1/PA2 , shall I perform some
> > transformation on the register address of the GPIO block or example ?
> > But how can I assure that if the next SoCFPGA has these addresses
> > completely different, these GPIO numbers will be stable ? Isn't it
> > better to be explicit about the GPIO block ID then ?
> 
> It's up to you. Normally each bank has a name and the datasheet
> specifies it. In your case if not you could think about a naming
> scheme.

Can you please take a look into arch/arm/dts/socfpga.dtsi ?
The system has three GPIO controllers (look for gpio0, gpio1, gpio2)
and each of these controllers has one bank (porta, portb, portc) .

I can name my gpios portxN , where x is either of a,b,c and N is the
GPIO number. The problem is, I cannot determine in dwapb_gpio_bind()
which one is "porta", "portb" and "portc" because all I have is the
physical addess of the GPIO controller and the index of the bank in
the namespace of that controller.

Sure, I can do some sort of global counting in the driver, but I would
like to avoid that sort of thing. I can also add some kind of ad-hoc DT
prop, but that's also not a good idea I think. Do you have any suggestion
for me please ?

> > Also, remember that I have an FPGA in the same package, which has a lot
> > of I/O.
> > 
> >> > Also, I can as well use "gpio <operation> N" , where N is a number.
> > 
> > What about this ? How does indexing the GPIOs with plain number fit into
> > the picture please ?
> 
> Driver model GPIO handles this automatically. If you can add different
> numbers of GPIO blocks you might consider adding them as different
> GPIO banks with their own names. The global number depends on the
> probe order which depends on the bind order (i.e. device tree node
> order). But names are safer than numbers - a small change can change
> all the numbers. There is a function to get the global number given a
> GPIO device and offset.

I see, thanks for clarifying!