[PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Stefan Wahren]

For years, the Raspberry Pi users relied on userspace programs to read
the pin configuration. In the meantime, it has become apparent that this
approach has reached its limits for various reasons.

This patch series now attempts to improve the debugging possibilities on
the kernel side in order to reduce the dependency on these userspace
programs.

Stefan Wahren (2):
  pinctrl: bcm2835: Implement bcm2835_pinconf_get
  pinctrl: bcm2835: Implement bcm2711_pinconf_get

 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 78 ++++++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

--
2.34.1

[PATCH RFC 1/2] pinctrl: bcm2835: Implement bcm2835_pinconf_get

[Stefan Wahren]

Even the driver already has implemented pin_dbg_show, it could
be helpful to implement pin_conf_get for a more generic behavior.
Contrary to the BCM2711, the BCM2835 SOC doesn't allow to read
the bias config, so the implementation is limited to the basics.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 35 ++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 1489191a213f..b37c86ec6915 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1003,7 +1003,40 @@ static const struct pinmux_ops bcm2835_pmx_ops = {
 static int bcm2835_pinconf_get(struct pinctrl_dev *pctldev,
 			unsigned pin, unsigned long *config)
 {
-	/* No way to read back config in HW */
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum bcm2835_fsel fsel = bcm2835_pinctrl_fsel_get(pc, pin);
+	u32 val;
+
+	/* No way to read back bias config in HW */
+
+	switch (param) {
+	case PIN_CONFIG_INPUT_ENABLE:
+		if (fsel != BCM2835_FSEL_GPIO_IN)
+			return -EINVAL;
+
+		*config = pinconf_to_config_packed(param, 1);
+		return 0;
+
+	case PIN_CONFIG_OUTPUT_ENABLE:
+		if (fsel != BCM2835_FSEL_GPIO_OUT)
+			return -EINVAL;
+
+		*config = pinconf_to_config_packed(param, 1);
+		return 0;
+
+	case PIN_CONFIG_OUTPUT:
+		if (fsel != BCM2835_FSEL_GPIO_OUT)
+			return -EINVAL;
+
+		val = bcm2835_gpio_get_bit(pc, GPLEV0, pin);
+		*config = pinconf_to_config_packed(param, val);
+		return 0;
+
+	default:
+		break;
+	}
+
 	return -ENOTSUPP;
 }

--
2.34.1

[PATCH RFC 2/2] pinctrl: bcm2835: Implement bcm2711_pinconf_get

[Stefan Wahren]

The BCM2711 allows to read the bias config. So implement pin_conf_get
accordingly. The pull resistor values has been taken from the CM4
datasheet.

This implementation assumes that BCM7211 behaves the same way.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 43 ++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index b37c86ec6915..fcb9ab6729c0 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1112,6 +1112,47 @@ static const struct pinconf_ops bcm2835_pinconf_ops = {
 	.pin_config_set = bcm2835_pinconf_set,
 };

+static int bcm2711_pinconf_get(struct pinctrl_dev *pctldev, unsigned pin,
+			       unsigned long *config)
+{
+	struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	u32 offset, shift, val;
+
+	offset = PUD_2711_REG_OFFSET(pin);
+	shift = PUD_2711_REG_SHIFT(pin);
+	val = bcm2835_gpio_rd(pc, GP_GPIO_PUP_PDN_CNTRL_REG0 + (offset * 4));
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		if (((val >> shift) & PUD_2711_MASK) == BCM2711_PULL_NONE)
+			return 0;
+		else
+			return -EINVAL;
+
+	case PIN_CONFIG_BIAS_PULL_UP:
+		if (((val >> shift) & PUD_2711_MASK) == BCM2711_PULL_UP) {
+			*config = pinconf_to_config_packed(param, 47000);
+			return 0;
+		} else {
+			return -EINVAL;
+		}
+
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		if (((val >> shift) & PUD_2711_MASK) == BCM2711_PULL_DOWN) {
+			*config = pinconf_to_config_packed(param, 47000);
+			return 0;
+		} else {
+			return -EINVAL;
+		}
+
+	default:
+		break;
+	}
+
+	return bcm2835_pinconf_get(pctldev, pin, config);
+}
+
 static void bcm2711_pull_config_set(struct bcm2835_pinctrl *pc,
 				    unsigned int pin, unsigned int arg)
 {
@@ -1179,7 +1220,7 @@ static int bcm2711_pinconf_set(struct pinctrl_dev *pctldev,

 static const struct pinconf_ops bcm2711_pinconf_ops = {
 	.is_generic = true,
-	.pin_config_get = bcm2835_pinconf_get,
+	.pin_config_get = bcm2711_pinconf_get,
 	.pin_config_set = bcm2711_pinconf_set,
 };

--
2.34.1

Re: [PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Linus Walleij]

On Sun, Feb 4, 2024 at 5:59 PM Stefan Wahren <wahrenst@gmx.net> wrote:

> For years, the Raspberry Pi users relied on userspace programs to read
> the pin configuration. In the meantime, it has become apparent that this
> approach has reached its limits for various reasons.
>
> This patch series now attempts to improve the debugging possibilities on
> the kernel side in order to reduce the dependency on these userspace
> programs.
>
> Stefan Wahren (2):
>   pinctrl: bcm2835: Implement bcm2835_pinconf_get
>   pinctrl: bcm2835: Implement bcm2711_pinconf_get

I don't see any problems with this, can I just apply the patches or do you
want to first resend them as non-RFC?

Yours,
Linus Walleij

Re: [PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Stefan Wahren]

Hi Linus,

Am 07.02.24 um 11:56 schrieb Linus Walleij:
> On Sun, Feb 4, 2024 at 5:59 PM Stefan Wahren <wahrenst@gmx.net> wrote:
>
>> For years, the Raspberry Pi users relied on userspace programs to read
>> the pin configuration. In the meantime, it has become apparent that this
>> approach has reached its limits for various reasons.
>>
>> This patch series now attempts to improve the debugging possibilities on
>> the kernel side in order to reduce the dependency on these userspace
>> programs.
>>
>> Stefan Wahren (2):
>>    pinctrl: bcm2835: Implement bcm2835_pinconf_get
>>    pinctrl: bcm2835: Implement bcm2711_pinconf_get
> I don't see any problems with this, can I just apply the patches or do you
> want to first resend them as non-RFC?
since the second patch hasn't been tested with BCM7211, it would be nice
to get a feedback from Florian or someone else with this hardware?

Best regards
>
> Yours,
> Linus Walleij
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On 2/7/24 03:33, Stefan Wahren wrote:
> Hi Linus,
> 
> Am 07.02.24 um 11:56 schrieb Linus Walleij:
>> On Sun, Feb 4, 2024 at 5:59 PM Stefan Wahren <wahrenst@gmx.net> wrote:
>>
>>> For years, the Raspberry Pi users relied on userspace programs to read
>>> the pin configuration. In the meantime, it has become apparent that this
>>> approach has reached its limits for various reasons.
>>>
>>> This patch series now attempts to improve the debugging possibilities on
>>> the kernel side in order to reduce the dependency on these userspace
>>> programs.
>>>
>>> Stefan Wahren (2):
>>>    pinctrl: bcm2835: Implement bcm2835_pinconf_get
>>>    pinctrl: bcm2835: Implement bcm2711_pinconf_get
>> I don't see any problems with this, can I just apply the patches or do 
>> you
>> want to first resend them as non-RFC?
> since the second patch hasn't been tested with BCM7211, it would be nice
> to get a feedback from Florian or someone else with this hardware?

Give me a few days to get there and I will run a test.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Stefan Wahren]

Hi Florian,

Am 07.02.24 um 19:22 schrieb Florian Fainelli:
> On 2/7/24 03:33, Stefan Wahren wrote:
>> Hi Linus,
>>
>> Am 07.02.24 um 11:56 schrieb Linus Walleij:
>>> On Sun, Feb 4, 2024 at 5:59 PM Stefan Wahren <wahrenst@gmx.net> wrote:
>>>
>>>> For years, the Raspberry Pi users relied on userspace programs to read
>>>> the pin configuration. In the meantime, it has become apparent that
>>>> this
>>>> approach has reached its limits for various reasons.
>>>>
>>>> This patch series now attempts to improve the debugging
>>>> possibilities on
>>>> the kernel side in order to reduce the dependency on these userspace
>>>> programs.
>>>>
>>>> Stefan Wahren (2):
>>>>    pinctrl: bcm2835: Implement bcm2835_pinconf_get
>>>>    pinctrl: bcm2835: Implement bcm2711_pinconf_get
>>> I don't see any problems with this, can I just apply the patches or
>>> do you
>>> want to first resend them as non-RFC?
>> since the second patch hasn't been tested with BCM7211, it would be nice
>> to get a feedback from Florian or someone else with this hardware?
>
> Give me a few days to get there and I will run a test.

is there any chance to test with BCM7211?

Re: [PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

On 2/25/24 02:01, Stefan Wahren wrote:
> Hi Florian,
> 
> Am 07.02.24 um 19:22 schrieb Florian Fainelli:
>> On 2/7/24 03:33, Stefan Wahren wrote:
>>> Hi Linus,
>>>
>>> Am 07.02.24 um 11:56 schrieb Linus Walleij:
>>>> On Sun, Feb 4, 2024 at 5:59 PM Stefan Wahren <wahrenst@gmx.net> wrote:
>>>>
>>>>> For years, the Raspberry Pi users relied on userspace programs to read
>>>>> the pin configuration. In the meantime, it has become apparent that
>>>>> this
>>>>> approach has reached its limits for various reasons.
>>>>>
>>>>> This patch series now attempts to improve the debugging
>>>>> possibilities on
>>>>> the kernel side in order to reduce the dependency on these userspace
>>>>> programs.
>>>>>
>>>>> Stefan Wahren (2):
>>>>>    pinctrl: bcm2835: Implement bcm2835_pinconf_get
>>>>>    pinctrl: bcm2835: Implement bcm2711_pinconf_get
>>>> I don't see any problems with this, can I just apply the patches or
>>>> do you
>>>> want to first resend them as non-RFC?
>>> since the second patch hasn't been tested with BCM7211, it would be nice
>>> to get a feedback from Florian or someone else with this hardware?
>>
>> Give me a few days to get there and I will run a test.
> 
> is there any chance to test with BCM7211?

Sorry about the delay, I just gave those patches a try although I am not 
sure where I should be seeing the reporting about being configured as 
pull up/down? Is this supposed to be visible under 
/sys/kernel/debug/pinctrl/*.gpio-pinctrl-bcm2711/pins?

Checking the datasheet for 2711/7211 we advertise 50kOhms internal pull 
ups/downs for all GPIO pins, so we might need to make the resistor value 
different based upon the board/SoC maybe.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH RFC 0/2] pinctrl: bcm2835: Implement pin_conf_get

[Stefan Wahren]

Hi Florian,

Am 27.02.24 um 01:22 schrieb Florian Fainelli:
> On 2/25/24 02:01, Stefan Wahren wrote:
>> Hi Florian,
>>
>> Am 07.02.24 um 19:22 schrieb Florian Fainelli:
>>> On 2/7/24 03:33, Stefan Wahren wrote:
>>>> Hi Linus,
>>>>
>>>> Am 07.02.24 um 11:56 schrieb Linus Walleij:
>>>>> On Sun, Feb 4, 2024 at 5:59 PM Stefan Wahren <wahrenst@gmx.net>
>>>>> wrote:
>>>>>
>>>>>> For years, the Raspberry Pi users relied on userspace programs to
>>>>>> read
>>>>>> the pin configuration. In the meantime, it has become apparent that
>>>>>> this
>>>>>> approach has reached its limits for various reasons.
>>>>>>
>>>>>> This patch series now attempts to improve the debugging
>>>>>> possibilities on
>>>>>> the kernel side in order to reduce the dependency on these userspace
>>>>>> programs.
>>>>>>
>>>>>> Stefan Wahren (2):
>>>>>>    pinctrl: bcm2835: Implement bcm2835_pinconf_get
>>>>>>    pinctrl: bcm2835: Implement bcm2711_pinconf_get
>>>>> I don't see any problems with this, can I just apply the patches or
>>>>> do you
>>>>> want to first resend them as non-RFC?
>>>> since the second patch hasn't been tested with BCM7211, it would be
>>>> nice
>>>> to get a feedback from Florian or someone else with this hardware?
>>>
>>> Give me a few days to get there and I will run a test.
>>
>> is there any chance to test with BCM7211?
>
> Sorry about the delay, I just gave those patches a try although I am
> not sure where I should be seeing the reporting about being configured
> as pull up/down? Is this supposed to be visible under
> /sys/kernel/debug/pinctrl/*.gpio-pinctrl-bcm2711/pins?
It should be .../pinconf-pins
>
> Checking the datasheet for 2711/7211 we advertise 50kOhms internal
> pull ups/downs for all GPIO pins, so we might need to make the
> resistor value different based upon the board/SoC maybe.
Thanks for this, in this case i would change the resistor values to
50000 Ohms. The CM4 datasheet actually provide a range.

Regards