RE: gpio-mcp23sxx driver stopped working

["Zhang, Sonic" <Sonic.Zhang@xxxxxxxxxx>]

Hi Antonio,

>-----Original Message-----
>From: Antonio Fiol Bonnín [mailto:antonio@xxxxxxx]
>Sent: Monday, January 19, 2015 6:55 PM
>To: Alexandre Courbot
>Cc: Zhang, Sonic; Linus Walleij; Grant Likely; Rob Herring; linux-gpio@xxxxxxxxxxxxxxx
>Subject: Re: gpio-mcp23sxx driver stopped working
>
>On Mon, Jan 19, 2015 at 7:08 AM, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:
>> On Mon, Jan 19, 2015 at 1:01 PM, Zhang, Sonic <Sonic.Zhang@xxxxxxxxxx> wrote:
>>> Hi Antonio,
>>>
>>>>-----Original Message-----
>>>>From: Antonio Fiol Bonnín [mailto:antonio@xxxxxxx]
>>>>Sent: Sunday, January 18, 2015 7:32 PM
>>>>To: Alexandre Courbot; Zhang, Sonic
>>>>Cc: Linus Walleij; Grant Likely; Rob Herring;
>>>>linux-gpio@xxxxxxxxxxxxxxx
>>>>Subject: Re: gpio-mcp23sxx driver stopped working
>>>>
>>>>And I have some more news...
>>>>
>>>>On Sun, Jan 18, 2015 at 9:34 AM, Antonio Fiol Bonnín <antonio@xxxxxxx> wrote:
>>>>> On Sun, Jan 18, 2015 at 7:04 AM, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:
>>>>>> On Sat, Jan 17, 2015 at 3:28 AM, Antonio Fiol Bonnín
>>>>>> <antonio@xxxxxxx>
>>>>>> wrote:
>>>>>> > I am writing to you as the maintainers for the gpio-mcp23sxx
>>>>>> > driver (and open firmware maintainers) according to the kernel
>>>>>> > documentation, before writing to the linux-kernel, linux-gpio or
>>>>>> > devicetree mailing list, as recommeded by the FAQ.  If you
>>>>>> > believe I should proceed otherwise, I will be pleased to do so.
>>>>>>
>>>>>> Thanks for reporting that issue and your detailed report. AFAICT
>>>>>> you did everything well - you could maybe also have included the
>>>>>> linux-gpio ML to reach a wider audience.
>>>>
>>>>OK, Adding the list now that I have some more info.
>>>>
>>>>>> > I was using that module on a raspberry pi on kernel 3.15.2,
>>>>>> > successfully.
>>>>>> > When I upgraded to 3.18.2, the module stopped working.
>>>>>> > Specifically, the probes for a MCP23017 device fail with -EINVAL (-22).
>>>>>> >
>>>>>> > Some background:
>>>>>> > The setup is as follows:
>>>>>> > Some RPi GPIO pins drive leds. These continue working. Unrelated
>>>>>> > to the failing module.
>>>>>> > RPi GPIO port 4 is used as the master for a w1 bus holding a
>>>>>> > couple temperature sensors. Still working properly. This is
>>>>>> > relevant because I am upgrading after observing some kernel oops
>>>>>> > related to the w1_therm module, but still unrelated to the failing module.
>>>>>> >
>>>>>> > The issue:
>>>>>> > RPI I2C port connected to a MCP23017 which, in turn, has 16 I/O
>>>>>> > pins configured to drive some more leds. This is still working
>>>>>> > at the I2C level, but the gpio-mcp23sxx module fails to probe
>>>>>> > the chip, and this means it fails to create the
>>>>>> > /sys/bus/gpio/gpiochip240 so I can't export the pins which I
>>>>>> > later use to change the led state, all this using sysfs.
>>>>>> >
>>>>>> > Some debugging:
>>>>>> > I checked that the driver is rejecting the probe because of
>>>>>> > "invalid platform data". I tracked it down to be "inexistent"
>>>>>> > platform data, as pdata is null when the test is performed.
>>>>>> > That code path is reached both when the kernel is configured
>>>>>> > with CONFIG_OF and when it is configured without it.
>>>>>> > Without it, I can understand, but with, I can't.
>>>>>> > The OF matching does not succeed because dev.of_node is null too.
>>>>>> >
>>> Have you added the gpio node for mcp23sxx in the device tree file of your raspberry pi board? And have you rebuilt your
>dtb file?
>
>No, I have not.
>
>> It seems like Antonio's device is using platform data (Antonio, can
>> you confirm?), so is DT relevant at all here?
>
>Well, both device tree and platform data are new concepts to me. So I don't quite know...
>
>> By the look of it this patch might have broken platform support for
>> this driver. According to the trace given by Antonio, the following
>> condition in probe() is met:
>>
>>         if (!pdata || !gpio_is_valid(pdata->base)) {
>>
>> Antonio, could you display both pdata and pdata->base right before
>> this test so we understand why this is happening?
>
>In the original failing version, as I explained above, pdata was NULL ("pdata is null when the test is performed")
>
>With the 3.15.2, when I
>echo mcp23017 0x27 > /sys/bus/i2c/devices/i2c-1/new_device
>it automatically created a "gpiochip240" under /sys/classes/gpio, and I could export the gpio pins 240-255.
>
>With 3.18.2 and the driver right before Sonic's patch (the latest working scenario), when I echo mcp23017 0x27 >
>/sys/bus/i2c/devices/i2c-1/new_device
>it automatically creates a "gpiochip496" under /sys/classes/gpio, and I can export the gpio pins 496-511.
>
>It seems like someone is assigning the "base" automatically, down from a certain maximum, that may have been increased
>since 3.15.2.
It seems the former mcp23017 driver create a default gpio base address and an irq number if you have neither a device node nor a platform data defined. Is it correct to skip both the device node and the platform data for a platform driver? If yes, I can send a patch to support this behavior.


Regards,

Sonic
��.n��������+%������w��{.n�����{��
b���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f