> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@xxxxxxxxxx] > Sent: 26 June 2024 18:31 > To: Vishnu Reddy <vishnu.reddy@xxxxxxxxxxx>; 'Krzysztof Kozlowski' > <krzk@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 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). Hi, Thanks for suggestion. I have gone through this and found that driver match data in this driver is stored in the __initconst section, which is freed up after kernel initialization. So we have two options: 1: Keep this platform specific data in driver match data and then populate driver_data field in probe function. 2: Use compatible matching and set different values during set_config. First approach will result in many changes, such as populating driver match data for all platforms and then storing the same in driver_data in probe. In the second approach, we can handle this using simple if/else based on a compatible match. IMO, second approach would be simpler and introduce less changes. Any suggestions from your end? > > Best regards, > Krzysztof
