RFI: pinctrl & mcp32s08 driver

RFI: pinctrl & mcp32s08 driver

[Phil Reid]

G'day All,

The mcp32s08 driver was recently moved from the drivers/gpio to drivers/pinctlr.

My system (SOCFPGA) doesn't have a pinctrl so it never set kconfig PINCTRL variable.
So I can't get access to enable the new PINCTRL_MCP23S08 variable to enable the driver.

Not sure what I'm supposed to do here. Documentation suggests pinctrl is platform specific,
but the mcp32s08 driver isn't really soc specific being an i2c/spi device.

For testing I've edited the pinctrl/Kconfig to make PINCTRL a menuconfig variable same as GPIOLIB


In addition my hardware has a mcp23017 & mcp23018 sharing an irq line.
So this requires an active low open drain output.
This line is then routed via fpga fabric to the SOCFPGA arm processor generic interrupt input
which only accepts active high inputs. So it's inverted in the fpga fabric.

The mcp23s08 driver in mcp23s08_irq_setup forces the irq flag based on the chip
configuration. Which breaks this.

ie setting IRQ_TYPE_LEVEL_HIGH in the device tree for the mcp irq is effectively ignored.

What's the correct approach here. Shouldn't this just honour the device tree?



Also noticed when compiling the driver in new mcp_pinconf_set()

	case PIN_CONFIG_BIAS_PULL_UP:
		val = arg ? 0xFFFF : 0x0000;
		mask = BIT(pin);
		ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
		break;

val & mask aren't used.

I'm not sure which part is the typo.



-- 
Regards
Phil Reid

Re: RFI: pinctrl & mcp32s08 driver

[Linus Walleij]

On Thu, Oct 5, 2017 at 8:54 AM, Phil Reid <preid@electromag.com.au> wrote:

> My system (SOCFPGA) doesn't have a pinctrl so it never set kconfig PINCTRL
> variable.
> So I can't get access to enable the new PINCTRL_MCP23S08 variable to enable
> the driver.
>
> Not sure what I'm supposed to do here. Documentation suggests pinctrl is
> platform specific,

Hm Torvalds had me hide the option because he don't want to be bothered
with stuff he don't care about.

I don't know if that is really appropriate now that we have pin controllers
on slow busses popping up.

A short term fix is just to select PINCTRL on SOCFPGA and ask the
ARM SoC maintainers what to do.

> but the mcp32s08 driver isn't really soc specific being an i2c/spi device.

Nope. But I guess mostly ARM devices use it?

> For testing I've edited the pinctrl/Kconfig to make PINCTRL a menuconfig
> variable same as GPIOLIB

Maybe you should propose that as a patch.
Arnd, what do you think?

> In addition my hardware has a mcp23017 & mcp23018 sharing an irq line.
> So this requires an active low open drain output.

Or an active high open source output. But OK.

> This line is then routed via fpga fabric to the SOCFPGA arm processor
> generic interrupt input
> which only accepts active high inputs. So it's inverted in the fpga fabric.
>
> The mcp23s08 driver in mcp23s08_irq_setup forces the irq flag based on the
> chip
> configuration. Which breaks this.
>
> ie setting IRQ_TYPE_LEVEL_HIGH in the device tree for the mcp irq is
> effectively ignored.
>
> What's the correct approach here. Shouldn't this just honour the device
> tree?

You need to model the inverter in the device tree and have something
pass it through I guess, but the IRQ maintainers are the ones to ask.
(Thomas Gleixner, Marc Zyngier)

> Also noticed when compiling the driver in new mcp_pinconf_set()
>
>         case PIN_CONFIG_BIAS_PULL_UP:
>                 val = arg ? 0xFFFF : 0x0000;
>                 mask = BIT(pin);
>                 ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
>                 break;
>
> val & mask aren't used.
>
> I'm not sure which part is the typo.

Looks like a bug.

Sebastian Reichel wrote this code and can surely figure out what's wrong.

Yours,
Linus Walleij