Re: Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Fri, Oct 09, 2020 at 11:15:57AM +0200, Martin Hundebøll wrote:
> On 16/04/2020 12.35, Linus Walleij wrote:
> > On Tue, Apr 7, 2020 at 7:38 PM Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

...

> This series caused probing of my PiFace2 to fail:

Thanks for report!

> > [    1.019604] mcp23s08: probe of spi0.0 failed with error -22

I suppose you are trying this on v5.9-rc8 with the same result?

This case without any error message and with -EINVAL points me to the only
possible call that may fail, i.e.

	ret = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
	if (ret)
		return ret;

Can you confirm this by adding a dev_info() call in between?

	ret = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
	if (ret) {
		dev_info(dev, "regmap init failed for %d@%u\n", type, addr);
		return ret;
	}

and share what it prints, please?

EINVAL can be returned either by wrong type (see switch-case inside that
function) or by devm_regmap_init().

> I tried to bisect, but some of the commits failed to compile:
> 
> > drivers/pinctrl/pinctrl-mcp23s08.c:959:39: error: 'mcp23s08_spi_of_match'
> undeclared (first use in this function); did you mean
> 'mcp23s08_i2c_of_match'?
> 
> >  959 |  match = of_match_device(of_match_ptr(mcp23s08_spi_of_match),
> &spi->dev);

At which patch this happens?

I compiled them individually but it might be slipped during rebase in between
of versions of the patch series.

> However, bisecting did narrow it down to a few commits:

You meant that revert of all of them helps?

Can you rather try to revert all patches against this driver until the one
which starts the series and bisect again? (Should be like 4-5 iterations only).

> > * 1ac30db20be2 - (refs/bisect/bad) pinctrl: mcp23s08: Make use of device
> properties (6 months ago) <Andy Shevchenko>
> 
> > * 88af89b52a1b -
> (refs/bisect/skip-88af89b52a1b7493f1e5ec165856b4c6767cf654) pinctrl:
> mcp23s08: Propagate error code from device_property_read_u32() (6 months
> ago) <Andy Shevchenko>
> 
> > * 0874758ecb2b -
> (refs/bisect/skip-0874758ecb2b3faa200a86dda45dbc29335f883e) pinctrl:
> mcp23s08: Refactor mcp23s08_spi_regmap_init() (6 months ago) <Andy
> Shevchenko>
> 
> > * 0521701c8d10 -
> (refs/bisect/skip-0521701c8d10f832a401cc7ebfa92bb73782d792) pinctrl:
> mcp23s08: Drop unused parameter in mcp23s08_probe_one() (6 months ago) <Andy
> Shevchenko>
> 
> > * d3da29b628a8 -
> (refs/bisect/skip-d3da29b628a86777d25c741c44b8af35f10020a0) pinctrl:
> mcp23s08: Consolidate SPI and I²C code (6 months ago) <Andy Shevchenko>
> 
> > * 84d02e785d34 -
> (refs/bisect/good-84d02e785d34be9363a825d696cca1f07fac2634) pinctrl:
> mcp23s08: Deduplicate IRQ chip filling (6 months ago) <Andy Shevchenko>
> 
> I'm using the rpi3 device tree (broadcom/bcm2837-rpi-3-b.dtb) overlayed with
> a configuration for the mcp23s08:

> /dts-v1/;
> /plugin/;
> / {
>         fragment@0 {
>                 target-path = "/soc/spi@7e204000";
>                 __overlay__ {
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&spi0_gpio7>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         status = "okay";
>                         gpio@0 {
>                                 compatible = "microchip,mcp23s17";
>                                 reg = <0>;
>                                 gpio-controller;
>                                 #gpio-cells = <2>;
>                                 microchip,spi-present-mask = <0x01>;
>                                 spi-max-frequency = <1000000>;
>                         };
>                 };
>         };

Okay, there is no IRQ chip here... But this should be fine I think.

> };

> I looked around the code a bit, and tried to compare it with v5.7, but
> didn't see any obvious candidates.

-- 
With Best Regards,
Andy Shevchenko