On Fri, Feb 24, 2023 at 4:42 PM Asmaa Mnebhi <asmaa@xxxxxxxxxx> wrote:
>
> > > > > > > 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)
> })
> }
So far I see no issues with the tables. Thanks for sharing!
> > > 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.
So far it seems the issue is in bgpio_init() that doesn't handle
ngpios properly. Maybe we need to fix this first?
Btw, have you considered using the gpio-regmap approach?
--
With Best Regards,
Andy Shevchenko
