Re: [PATCH v2] pinctrl: samsung: Add support for pull-up and pull-down

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 26/06/2024 13:49, Vishnu Reddy wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzk@xxxxxxxxxx]
>> Sent: 24 June 2024 19:27
>> To: Vishnu Reddy <vishnu.reddy@xxxxxxxxxxx>;
>> krzysztof.kozlowski@xxxxxxxxxx; s.nawrocki@xxxxxxxxxxx;
>> alim.akhtar@xxxxxxxxxxx; linus.walleij@xxxxxxxxxx
>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung-
>> soc@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx; pankaj.dubey@xxxxxxxxxxx;
>> ravi.patel@xxxxxxxxxxx; gost.dev@xxxxxxxxxxx
>> Subject: Re: [PATCH v2] pinctrl: samsung: Add support for pull-up and pull-
>> down
>>
>> On 20/06/2024 12:34, Vishnu Reddy wrote:
>>> gpiolib framework has the implementation of setting up the
>>> PUD configuration for GPIO pins but there is no driver support.
>>>
>>> Add support to handle the PUD configuration request from the
>>> userspace in samsung pinctrl driver.
>>>
>>> Signed-off-by: Vishnu Reddy <vishnu.reddy@xxxxxxxxxxx>
>>> ---
>>> Verified the offset from the user manual of following Exynos SoC series
>>> and found the current code is taking care of correct offset for pull-up
>>> and pull-down
>>>
>>> Exynos-3250
>>> Exynos-3470
>>> Exynos-4412
>>> Exynos-4415
>>> Exynos-5250
>>> Exynos-5260
>>> Exynos-5410
>>> Exynos-5420
>>> Exynos-5422
>>> Exynos-7420
>>> Exynos-7580
>>> Exynos-7880
>>> Exynos-9820
>>> Exynos-9830
>>> Exynos-4210
>>> Exynos-S5PC210
>>> Exynos-S5PV310
>>>
>>> This patch is tested on FSD platform
>>
>> You verified but...
>>
>>> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h
>> b/drivers/pinctrl/samsung/pinctrl-samsung.h
>>> index d50ba6f07d5d..758b623a4bea 100644
>>> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h
>>> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
>>> @@ -61,6 +61,13 @@ enum pincfg_type {
>>>  #define PIN_CON_FUNC_INPUT		0x0
>>>  #define PIN_CON_FUNC_OUTPUT		0x1
>>>
>>> +/*
>>> + * Values for the pin PUD register.
>>> + */
>>> +#define PIN_PUD_PULL_UP_DOWN_DISABLE	0x0
>>> +#define PIN_PUD_PULL_DOWN_ENABLE	0x1
>>> +#define PIN_PUD_PULL_UP_ENABLE		0x3
>>
>> ... I said it is not correct, so you send the same? If you think I was
>> wrong, then please respond and keep discussion going. Sending the same
>> suggests you just ignored my comment.
>>
>> Look at two headers s5pv210-pinctrl.h and s3c64xx-pinctrl.h. How did you
>> resolve these?
> Thank you for sharing the s5pv210-pinctrl.h and s3c64xx-pinctrl.h  file names for the pin value information.
> I have not ignored your comment. Unfortunately, I don't have the user manuals for the s3c64xx and s5pv210 series.
> I have an idea to handle the PIN_PULL_UP value of the s3c64xx and s5pv210 series by checking the compatibility with the of_device_is_compatible API.
> Will it be okay or do you have any other suggestions?

I don't remember the code used here, but usually such choices are
determined by driver match data (and flags or value customized per variant).

Best regards,
Krzysztof