[U-Boot] [U-BOOT PATCH v1 0/2] add gpio support for HiFive Unleashed A00 board.

[Bin Meng]

Hi Sagar,

On Thu, Sep 26, 2019 at 1:54 AM Sagar Kadam <sagar.kadam at sifive.com> wrote:
>
> Hi Bin,
>
> On Wed, Sep 18, 2019 at 1:23 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Sagar,
> >
> > On Tue, Sep 10, 2019 at 11:44 PM Sagar Shrikant Kadam
> > <sagar.kadam at sifive.com> wrote:
> > >
> > > U-Boot currently is missing GPIO support for FU540-C000 SoC which is
> > > mounted on HiFive Unleashed A00 board. This patch is intended to add DM
> > > based GPIO controller driver in order to access GPIO pins within the SoC
> > > using GPIO command in U-Boot. More details on the GPIO controller within
> > > the SoC can be found at[1]
> > >
> > > The driver is based above master branch of u-boot-riscv.git and provides a
> > > method to configure Input/Output mode of the GPIO pin along with an option
> > > to set or clear state of the GPIO pin. The patch is available in
> > > dev/sagark/gpio_v3 branch here[2].
> > >
> > > GPIO device node added to the mainline bound device tree for HiFive
> > > Unleashed is available in dev/sagark/mlv5.3-rc5 branch of repo here[3].
> > >
> > > This implementation is ported from linux driver submitted for review
> > > at [4].
> > >
> > > More details of GPIO pin routing on J1 header is available in schematic
> > > document[5]
> > >
> > > [1] https://static.dev.sifive.com/FU540-C000-v1.0.pdf
> > > [2] https://github.com/sagsifive/u-boot
> > > [3] https://github.com/sagsifive/riscv-linux-hifive/
> > > [4] https://lkml.org/lkml/2018/10/9/1103
> > > [5] https://static.dev.sifive.com/dev-kits/hifive-unleashed/hifive-unleashed-a00-schematics.pdf
> > >
> > > Driver Testing:
> > > #Set GPIO1 high.
> > > =>gpio set 1
> > >   Can be confirmed by probing pin No #24 on J1 Header or memory dump of
> > >   gpio register space viz: #md 0x10060000
> > >
> > > #Set GPIO1 low
> > > =>gpio clear 0
> > >
> > > #Toggle GPIO1
> > > =>gpio toggle 1         #Toggle value of GPIO1
> > > =>gpio toggle 1         #Toggle value of GPIO1
> > >
> > > #Configure pin as input
> > > =>gpio input 3          #Configure gpio line 3 as input.
> > >
> > > #Error check
> > > =>gpio set 16           #Not a valid GPIO number for FU540-C000
> > >   GPIO: '16' not found
> > >   Command 'gpio' failed: Error -22
> > >
> >
> > I tested this:
> >
> > => gpio status -a
> > Bank gpio at 10060000:
> > gpio at 100600000: unknown
> > gpio at 100600001: unknown
> > gpio at 100600002: unknown
> > gpio at 100600003: unknown
> > gpio at 100600004: unknown
> > gpio at 100600005: unknown
> > gpio at 100600006: unknown
> > gpio at 100600007: unknown
> > gpio at 100600008: unknown
> > gpio at 100600009: unknown
> > gpio at 1006000010: unknown
> > gpio at 1006000011: unknown
> > gpio at 1006000012: unknown
> > gpio at 1006000013: unknown
> > gpio at 1006000014: unknown
> > gpio at 1006000015: unknown
> >
> > The status is "unknown" for all gpio pins, which is wrong. It should
> > be either input or output.
>
> Thank you for your suggestions.
> The get_function operation is missing for this driver and so the
> status is unknown.
> I will implement it and send a revised version. Thanks for catching this.
> Please correct me if I am wrong, what I do see is that the gpio command
> uses the bank name appended before the GPIO number. So the bank_name
> as assigned in the driver probe function gets prefixed to the pin number and
> so it shows:
> gpio at 100600000
> gpio at 100600001
> and so on.
> I see that few driver's updates the uc_priv->bank_name in probe function
> with '_' as the separator between bank_name and pin number and so
> #gpio status -a will show it as :
>
> Bank <bank_name>:
> <bank_name>_0: input : 1 []
> <bank_name>_1: input : 1 []
> ........ and so on
>
> eg: In the current case here it will show as
> Bank gpio at 10060000_:
> gpio at 10060000_0
> gpio at 10060000_1 and so on.
>
> Please let me know if this implementation is ok.
> >
> > Also the gpio pin name is weird. I think we should use "0, 1, 2 ..."
> >
> The current implementation of the gpio_get_status function includes
> the base_name
> to the pin description. Truncating it here can help to get pin numbers
> as just numbers
> "0,1,2....". I will also include this if needed?
>

I think <bank_name>_0 is fine. Thanks!

Regards,
Bin