linux-kernel - RE: [PATCH v2] pinctrl: samsung: Add support for pull-up and pull-down

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 27 Jun 2024 19:05:41 +0530
From: "Vishnu Reddy" <vishnu.reddy@...sung.com>
To: "'Krzysztof Kozlowski'" <krzysztof.kozlowski@...aro.org>, "'Krzysztof
 Kozlowski'" <krzk@...nel.org>, <s.nawrocki@...sung.com>,
	<alim.akhtar@...sung.com>, <linus.walleij@...aro.org>
Cc: <linux-arm-kernel@...ts.infradead.org>,
	<linux-samsung-soc@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <pankaj.dubey@...sung.com>,
	<ravi.patel@...sung.com>, <gost.dev@...sung.com>
Subject: RE: [PATCH v2] pinctrl: samsung: Add support for pull-up and
 pull-down



> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@...aro.org]
> Sent: 26 June 2024 18:31
> To: Vishnu Reddy <vishnu.reddy@...sung.com>; 'Krzysztof Kozlowski'
> <krzk@...nel.org>; s.nawrocki@...sung.com; alim.akhtar@...sung.com;
> linus.walleij@...aro.org
> Cc: linux-arm-kernel@...ts.infradead.org; linux-samsung-
> soc@...r.kernel.org; linux-gpio@...r.kernel.org; linux-
> kernel@...r.kernel.org; pankaj.dubey@...sung.com;
> ravi.patel@...sung.com; gost.dev@...sung.com
> 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@...nel.org]
> >> Sent: 24 June 2024 19:27
> >> To: Vishnu Reddy <vishnu.reddy@...sung.com>;
> >> krzysztof.kozlowski@...aro.org; s.nawrocki@...sung.com;
> >> alim.akhtar@...sung.com; linus.walleij@...aro.org
> >> Cc: linux-arm-kernel@...ts.infradead.org; linux-samsung-
> >> soc@...r.kernel.org; linux-gpio@...r.kernel.org; linux-
> >> kernel@...r.kernel.org; pankaj.dubey@...sung.com;
> >> ravi.patel@...sung.com; gost.dev@...sung.com
> >> 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@...sung.com>
> >>> ---
> >>> 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