mbox series

[v1,0/5] pinctrl: intel: High impedance impl. and cleanups

Message ID 20240828184018.3097386-1-andriy.shevchenko@linux.intel.com
Headers show
Series pinctrl: intel: High impedance impl. and cleanups | expand

Message

Andy Shevchenko Aug. 28, 2024, 6:38 p.m. UTC
We would need a high impedance implementation for a quirk, so here it
is. While doing this series I also noticed a couple of opportunities
to clean up, hence two more patches (1st and 5th).

Andy Shevchenko (5):
  pinctrl: intel: Move debounce validation out of the lock
  pinctrl: intel: Refactor __intel_gpio_set_direction() to be more
    useful
  pinctrl: intel: Add __intel_gpio_get_direction() helper
  pinctrl: intel: Implement high impedance support
  pinctrl: intel: Introduce for_each_intel_gpio_group() helper

 drivers/pinctrl/intel/pinctrl-intel.c | 239 ++++++++++++++++----------
 1 file changed, 150 insertions(+), 89 deletions(-)

Comments

Andy Shevchenko Aug. 28, 2024, 8 p.m. UTC | #1
+Cc: Heiner

On Wed, Aug 28, 2024 at 09:38:33PM +0300, Andy Shevchenko wrote:
> We would need a high impedance implementation for a quirk, so here it
> is. While doing this series I also noticed a couple of opportunities
> to clean up, hence two more patches (1st and 5th).

Sorry it took a while to actually start implementing the quirk for your case.
Here I'm asking for the following things:

1) what is the marketing name of the device you have problems with?
(I believe it's available on the free market, correct?);

2) does it have any BIOS updates and, if it has, does it fix the issue?

3) can you apply patches 2,3,4,5 from this series (the first one is buggy and
not needed for you) and replace the hack I mentioned earlier with

	ret = intel_gpio_set_high_impedance(pctrl, 3);
	if (ret)
		return ret;

somewhere at the end of intel_pinctrl_probe()?

Does it still work as expected?
Heiner Kallweit Aug. 30, 2024, 5:56 a.m. UTC | #2
On 28.08.2024 22:00, Andy Shevchenko wrote:
> +Cc: Heiner
> 
> On Wed, Aug 28, 2024 at 09:38:33PM +0300, Andy Shevchenko wrote:
>> We would need a high impedance implementation for a quirk, so here it
>> is. While doing this series I also noticed a couple of opportunities
>> to clean up, hence two more patches (1st and 5th).
> 
> Sorry it took a while to actually start implementing the quirk for your case.
> Here I'm asking for the following things:
> 
> 1) what is the marketing name of the device you have problems with?
> (I believe it's available on the free market, correct?);
> 

Device is a dirt-cheap mini pc, marketed as Chatreey T9. It's available
on the free market, right. Dmesg says:
DMI: Default string Default string/Default string, BIOS ADLN.M6.SODIMM.ZB.CY.015 08/08/2023

> 2) does it have any BIOS updates and, if it has, does it fix the issue?
> 
No BIOS updates.

> 3) can you apply patches 2,3,4,5 from this series (the first one is buggy and
> not needed for you) and replace the hack I mentioned earlier with
> 
> 	ret = intel_gpio_set_high_impedance(pctrl, 3);
> 	if (ret)
> 		return ret;
> 
> somewhere at the end of intel_pinctrl_probe()?
> 
> Does it still work as expected?
> 
> 
I will check.
Andy Shevchenko Aug. 30, 2024, 6:51 p.m. UTC | #3
On Fri, Aug 30, 2024 at 07:56:11AM +0200, Heiner Kallweit wrote:
> On 28.08.2024 22:00, Andy Shevchenko wrote:
> > On Wed, Aug 28, 2024 at 09:38:33PM +0300, Andy Shevchenko wrote:
> >> We would need a high impedance implementation for a quirk, so here it
> >> is. While doing this series I also noticed a couple of opportunities
> >> to clean up, hence two more patches (1st and 5th).
> > 
> > Sorry it took a while to actually start implementing the quirk for your case.
> > Here I'm asking for the following things:
> > 
> > 1) what is the marketing name of the device you have problems with?
> > (I believe it's available on the free market, correct?);
> 
> Device is a dirt-cheap mini pc, marketed as Chatreey T9. It's available
> on the free market, right. Dmesg says:
> DMI: Default string Default string/Default string, BIOS ADLN.M6.SODIMM.ZB.CY.015 08/08/2023
> 
> > 2) does it have any BIOS updates and, if it has, does it fix the issue?
> > 
> No BIOS updates.
> 
> > 3) can you apply patches 2,3,4,5 from this series (the first one is buggy and
> > not needed for you) and replace the hack I mentioned earlier with
> > 
> > 	ret = intel_gpio_set_high_impedance(pctrl, 3);
> > 	if (ret)
> > 		return ret;
> > 
> > somewhere at the end of intel_pinctrl_probe()?
> > 
> > Does it still work as expected?
> > 
> I will check.

There is a v2 to test, you can take entire series, or for-next branch of Intel
pin control tree

https://git.kernel.org/pub/scm/linux/kernel/git/pinctrl/intel.git/log/?h=for-next

Thanks!
Heiner Kallweit Aug. 30, 2024, 9:24 p.m. UTC | #4
On 28.08.2024 22:00, Andy Shevchenko wrote:
> +Cc: Heiner
> 
> On Wed, Aug 28, 2024 at 09:38:33PM +0300, Andy Shevchenko wrote:
>> We would need a high impedance implementation for a quirk, so here it
>> is. While doing this series I also noticed a couple of opportunities
>> to clean up, hence two more patches (1st and 5th).
> 
> Sorry it took a while to actually start implementing the quirk for your case.
> Here I'm asking for the following things:
> 
> 1) what is the marketing name of the device you have problems with?
> (I believe it's available on the free market, correct?);
> 
> 2) does it have any BIOS updates and, if it has, does it fix the issue?
> 
> 3) can you apply patches 2,3,4,5 from this series (the first one is buggy and
> not needed for you) and replace the hack I mentioned earlier with
> 
> 	ret = intel_gpio_set_high_impedance(pctrl, 3);
> 	if (ret)
> 		return ret;
> 
> somewhere at the end of intel_pinctrl_probe()?
> 
> Does it still work as expected?
> 
> 
In latest series the return value of intel_gpio_set_high_impedance()
has been removed. With the call to intel_gpio_set_high_impedance()
changed accordingly this fixes the problem on my system.
Thanks a lot for your efforts!
Andy Shevchenko Sept. 2, 2024, 10:46 a.m. UTC | #5
On Fri, Aug 30, 2024 at 11:24:06PM +0200, Heiner Kallweit wrote:
> On 28.08.2024 22:00, Andy Shevchenko wrote:
> > On Wed, Aug 28, 2024 at 09:38:33PM +0300, Andy Shevchenko wrote:
> >> We would need a high impedance implementation for a quirk, so here it
> >> is. While doing this series I also noticed a couple of opportunities
> >> to clean up, hence two more patches (1st and 5th).
> > 
> > Sorry it took a while to actually start implementing the quirk for your case.
> > Here I'm asking for the following things:
> > 
> > 1) what is the marketing name of the device you have problems with?
> > (I believe it's available on the free market, correct?);
> > 
> > 2) does it have any BIOS updates and, if it has, does it fix the issue?
> > 
> > 3) can you apply patches 2,3,4,5 from this series (the first one is buggy and
> > not needed for you) and replace the hack I mentioned earlier with
> > 
> > 	ret = intel_gpio_set_high_impedance(pctrl, 3);
> > 	if (ret)
> > 		return ret;
> > 
> > somewhere at the end of intel_pinctrl_probe()?
> > 
> > Does it still work as expected?
> > 
> > 
> In latest series the return value of intel_gpio_set_high_impedance()
> has been removed. With the call to intel_gpio_set_high_impedance()
> changed accordingly this fixes the problem on my system.
> Thanks a lot for your efforts!

Thank you! I will think now, how to make the real quirk (which I may ask you to
test in the future, when it will be ready) that looks and feels not as an ugly
hack.