All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: "Niedermayr, BENEDIKT" <benedikt.niedermayr@siemens.com>
Cc: "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"haojian.zhuang@linaro.org" <haojian.zhuang@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org
Subject: Re: Pinconf issues on AMxxx plattforms
Date: Thu, 4 May 2023 08:35:09 +0300	[thread overview]
Message-ID: <20230504053509.GN14287@atomide.com> (raw)
In-Reply-To: <aa493d62327f26e4c65d649a812346cdfb26771f.camel@siemens.com>

Hi,

Adding linux-gpio and Linus W to Cc. Some questions and comments below
towards the end.

* Niedermayr, BENEDIKT <benedikt.niedermayr@siemens.com> [230503 08:38]:
> Hello,
> 
> We encountered some issues when accessing the gpiochardev interface on an
> AM65xx plaform.
>  
> The basic idea was to fully rely on all features the gpiochardev seems to
> offer. 
> I got all relevant information from the Linux Kernel Documentation
> (Documentation/driver-api/pin-control.rst) and saw 
> some presentations from Linus Walleij  regarding the gpiochardev
> capabilities.
> 
> If I understand that correctly the gpiochardev interface should support the
> following features:
> * Requesting gpio pins from userspace
> * Set input/output directions
> * Set BIAS settings (pull-up, pull-down, etc.)
> * Gpio function of that pin automatically gets muxed in if requested 
> 
> Requesting pins worked for me as expected after I added the required DTB
> properties:
> * pinctrl-single: Add each required pin to "pinctrl-single,gpio-range" in
> the pincontroller node 
> * gpio: Add each required pin to gpio-range property in the gpio node
> 
> I also added the required childnodes in the pinctrl node:
> 
> &main_pmx0 {
>   ... 
>    d6_gpio: d6-gpio {
> pinctrl-single,pins = <
> /* (AH16) GPIO0_38 */
> AM65X_IOPAD(0x0098, PIN_INPUT, 7)
> >;
> pinctrl-single,bias-pullup   = <0x20000  0x20000  0x10000  0x30000>;
> pinctrl-single,bias-pulldown = <0x00000  0x0      0x10000  0x30000>;
> };
> ...
> }
> 
> When I tried to set any BIAS settings nothing happened or some error occured
> in the kernel logs (i'm not 100% sure anymore since almost 2 months have
> past). 
> The first thing I had to do was to assign the gpiochip_generic_config
> callback to the gpiochiop for that (gpio-davinci.c). This callback in turn
> will finally call pcs_pinconf_set(), which
> is the pinctrl drivers related callback for setting the pinconf.
> But still no success...
> 
> Then I went deeper into the rabbit whole and encountered that the error had
> to do with pcs_get_function() (pinctrl-single.c).
> I found out that this function requests the current function (or pinmux
> state) from the pinctrl subsystem core. 
> The pinctrl driver needs this information for accessing the correct pinctrl
> childnode bits. 
> 
> So what is the Problem here?
> The pinctrl offers 3 different options for muxing:
> 
> 1.  Using the generic kernel APIs: 
>      Call pinctrl_select_state() function as stated
> in  Documentation/driver-api/pin-control.rst (section "Pin control requests
> from drivers").
>      This function will select a defined state which has been defined in DTB
> with "pinctrl-0", "pinctrl-1", "pinctrl-x"
> 2.  Mux pins with debugfs:
>      Write the desired pingroup and pinfunction into the "pinmux-select"
> file of the related pin controller.
> 3. Mux the GPIO function of a requested GPIO pin by calling the pinctrl
> drivers pcs_request_gpio() function.
> 
> The problem now is that only option 1. will store the current mux
> information in the pinctrl subsystems core. 
> The pinctrl-single driver highly depends on that information, which is not
> available at all wenn muxing with options 2&3.
> 
> I was able to fix that for option 2 but not for option 3. The problem here
> is that the pcs_request_gpio() function just does not provide enough
> parameters with sufficient information for achieving that task.

Care to post what your fix for #2 above looks like?

> I'm not sure if I miss something important here?
> Are you aware of this issue? 

Sounds like something needs to be implemented for pinctrl-single.c.

Regards,

Tony

  reply	other threads:[~2023-05-04  5:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-03  8:38 Pinconf issues on AMxxx plattforms Niedermayr, BENEDIKT
2023-05-04  5:35 ` Tony Lindgren [this message]
2023-05-04  8:31   ` Linus Walleij
2023-05-05  8:54     ` Tony Lindgren
2023-05-05 13:28     ` Niedermayr, BENEDIKT
2023-05-05 12:59   ` Niedermayr, BENEDIKT

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230504053509.GN14287@atomide.com \
    --to=tony@atomide.com \
    --cc=benedikt.niedermayr@siemens.com \
    --cc=haojian.zhuang@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.