Use of pinctrl-single for external device over I2C

Hi,

I am working on enabling support for PMIC 88PM860 device in the mainline.

In 88PM860 (and family of devices) few pins are labelled as GPIO's, to be precise, in 88PM860, we have 8 GPIO's (ana & dig).

I was looking at pinctrl-single driver, as it seems it can not handle pinmux configuration of external device (in this case its over I2C), as it uses raw read/write api's.

I see below lines in the driver,

/*
* REVISIT: Reads and writes could eventually use regmap or something
* generic. But at least on omaps, some mux registers are performance
* critical as they may need to be remuxed every time before and after
* idle. Adding tests for register access width for every read and
* write like regmap is doing is not desired, and caching the registers
* does not help in this case.
*/

Should be not have flag for this and use regmap_ variants? If we implement flag based approach then same driver can be reused for pinmux configuration of external device.

Just to give more clarity, Let me describe my use-case below,

The platform which I have is based on PXA1928 and 88PM860 chipsets, where 88PM860.GPIO_0 is connected back to PXA1928.EXT_32K_IN.

GPIO_0 need to configured in mode '4'.

As per spec, 88PM860.GPIO_0 can be configured to

000 = GPIO input mode
001 = GPIO output mode
010 = SLEEPOUTN mirror mode
011 = Buck4 FPWM enable
100 = 32 Khz output buffer mode
101 = PMICINTN output mode
110 = HW_RESET1 mode
111 = HW_RESET2 mode

Please let me know if there is already an alternative for this, which I missed.

Thanks,
Vaibhav

Thanks,
Vaibhav

Quoted 11 lines · Vaibhav Hiremath

/*
* REVISIT: Reads and writes could eventually use regmap or something
* generic. But at least on omaps, some mux registers are performance
* critical as they may need to be remuxed every time before and after
* idle. Adding tests for register access width for every read and
* write like regmap is doing is not desired, and caching the registers
* does not help in this case.
*/

Quoted 3 lines

Should be not have flag for this and use regmap_ variants? If we implement flag based approach then same driver can be reused for pinmux configuration of external device.

Quoted 12 lines · Vaibhav Hiremath

Quoted 11 lines · Vaibhav Hiremath

/*
* REVISIT: Reads and writes could eventually use regmap or something
* generic. But at least on omaps, some mux registers are performance
* critical as they may need to be remuxed every time before and after
* idle. Adding tests for register access width for every read and
* write like regmap is doing is not desired, and caching the registers
* does not help in this case.
*/

Quoted 3 lines

Quoted 3 lines

Should be not have flag for this and use regmap_ variants? If we implement flag based approach then same driver can be reused for pinmux configuration of external device.

Nothing stopping you from adding regmap support to it. It just needs to be made optional as the users so far don't need it.

I do not like this, as this is not HW feature, so DT may not be right approach.

So I will dig more from either runtime or Compile time option to use regmap_ Vs raw read/writes.

Quoted 6 lines · Vaibhav Hiremath

I do not like this, as this is not HW feature, so DT may not be right approach.

So I will dig more from either runtime or Compile time option to use regmap_ Vs raw read/writes.

Can't you just check if the pinctrl node has compatible = "syscon" property?

A compile time option won't work for sure. I don't know what you would check at runtime as you do not know what the bus is behind syscon.

Quoted 8 lines · Tony Lindgren

Quoted 6 lines · Vaibhav Hiremath

I do not like this, as this is not HW feature, so DT may not be right approach.

So I will dig more from either runtime or Compile time option to use regmap_ Vs raw read/writes.

Quoted 7 lines

Can't you just check if the pinctrl node has compatible = "syscon" property?

A compile time option won't work for sure. I don't know what you would check at runtime as you do not know what the bus is behind syscon.

Although, I haven't gone through syscon, but not sure whether syscon would be useful.

As you rightly stated, we need to know the bus behind regmap.

Quoted 19 lines · Vaibhav Hiremath

Quoted 8 lines · Tony Lindgren

Quoted 6 lines · Vaibhav Hiremath

I do not like this, as this is not HW feature, so DT may not be right approach.

So I will dig more from either runtime or Compile time option to use regmap_ Vs raw read/writes.

Quoted 7 lines

Can't you just check if the pinctrl node has compatible = "syscon" property?

A compile time option won't work for sure. I don't know what you would check at runtime as you do not know what the bus is behind syscon.

Quoted 5 lines

Although, I haven't gone through syscon, but not sure whether syscon would be useful.

As you rightly stated, we need to know the bus behind regmap.

Trying to understand what is the right way of doing pinctrl of external device on board,

I feel it would not be good idea to pollute pinctrl-single driver, and also I am still not able to figure out how can I have access to bus behind regmap.

How about having separate driver (generic for all I2C), say pinctrl- i2c.c, which is i2c_client driver and would support pinctrl and pinmux on I2C client device.

The current usecase which I have is pretty simple in nature,

88PM860 has few GPIO pins which can be configured to different functionality, based on board design.
In most of the cases they are one/init/boot time settings.

GPIO_0:
=======
000 = GPIO input mode
001 = GPIO output mode
010 = SLEEPOUTN mirror mode
011 = Buck4 FPWM enable
100 = 32 Khz output buffer mode
101 = PMICINTN output mode
110 = HW_RESET1 mode
111 = HW_RESET2 mode

Thanks,
Vaibhav