linux-kernel - RE: [PATCH v4 1/2] gpio: gpio-mlxbf3: Add gpio driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Feb 2023 14:42:56 +0000
From:   Asmaa Mnebhi <asmaa@...dia.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [PATCH v4 1/2] gpio: gpio-mlxbf3: Add gpio driver support

> > > > > > Ah that’s my bad. The property should be called "ngpios" like
> > > > > > in the DT
> > > > > documentation. Will fix.
> > > > >
> > > > > And why do you need it? What's a corner case that the GPIO
> > > > > library doesn't handle yet?
> > > >
> > > > We have 2 gpiochips, gpiochip 0 supports 32 gpio pins and gpiochip
> > > > 1
> > > supports only 24 pins.
> > > > If I remove the logic from gpio-mlxbf3.c, the gpiolib.c logic will
> > > > correctly set
> > > the ngpios = 32 for gpiochip 0 but will wrongly set ngpios=32 for gpiogchip
> 1:
> > >
> > > So, either you need to have two entries in DT per chip or ngpios should be
> 56.
> > >
> > I already have 2 entries in my ACPI table, in the first entry, ngpios = 32 and
> in the second entry ngpios = 24.
> 
> Can you show the DSDT excerpt of this device? (Also including the pieces for
> pin control)

Our ACPI tables are internal:

Device(GPI0) {
        Name(_HID, "MLNXBF33")
        Name(_UID, Zero)
        Name(_CCA, 1)
        Name(_CRS, ResourceTemplate() {
          // for fw_gpio[0] yu block
         Memory32Fixed(ReadWrite, 0x13401100, 0x00000080)
         // for yu_gpio_causereg0 block
         Memory32Fixed(ReadWrite, 0x13401c00, 0x00000020)
         Interrupt(ResourceConsumer, Edge, ActiveHigh, Shared)
           { BF_RSH0_YU }
       })
       Name(_DSD, Package() {
         ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
         Package() {
           Package () { "ngpios", 32 }, // Number of GPIO pins on gpio block 0
           Package () { "gpio-reserved-ranges", Package () {5, 1, 7, 3, 11, 31}},
         }
       })
     }
 Device(GPI1) {
       Name(_HID, "MLNXBF33")
       Name(_UID, 1)
       Name(_CCA, 1)
       Name(_CRS, ResourceTemplate() {
         // for fw_gpio[1] yu block
         Memory32Fixed(ReadWrite, 0x13401180, 0x00000080)
         // for yu_gpio_causereg1 block
         Memory32Fixed(ReadWrite, 0x13401c20, 0x00000020)
       })
       Name(_DSD, Package() {
         ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
         Package() {
           Package () { "ngpios", 24 }, // Number of GPIO pins on gpio block 1
           Package () { "gpio-reserved-ranges", Package () {0, 21}},
         }
       })
  }

    Device(PIN0) {
      Name(_HID, "MLNXBF34")
      Name(_UID, Zero)
      Name(_CCA, 1)
      Name(_CRS, ResourceTemplate() {
        // for fw_gpio[0] and fw_gpio[1] yu blocks
        Memory32Fixed(ReadWrite, 0x13401100, 0x00000100)
      })
     }

> 
> > Gpiochip_add_data_with_keys only reads the ngpios property if (ngpios
> > == 0) which is not the case when bgpio_init is called. bgpio_init uses "sz"
> argument to populate the ngpio in bgpio_init, which is not what we want.
> 
> Maybe bgpio_init() is not a good API for your case?


At the moment, bgpio_init handles all the direction in/out get/set gpio value for us.
So I can either remove bgpio_init so that we get the correct ngpios property, but will have to define get_gpio, set_gpio, dir in and dir out.
Or I can keep bgpio_init and device_property_read_u32 in the gpio-mlxbf3.c driver.
Please let me know which approach you prefer.