LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Piyush Mehta <piyushm@xilinx.com>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"zou_wei@huawei.com" <zou_wei@huawei.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	Michal Simek <michals@xilinx.com>,
	Jiaying Liang <jliang@xilinx.com>,
	"iwamatsu@nigauri.org" <iwamatsu@nigauri.org>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	Rajan Vaja <RAJANV@xilinx.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Srinivas Goud <sgoud@xilinx.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	git <git@xilinx.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH V3 3/3] gpio: modepin: Add driver support for modepin GPIO controller
Date: Wed, 18 Aug 2021 15:05:28 +0200	[thread overview]
Message-ID: <a1bd7d9f-5746-2d4c-8658-beb804468846@pengutronix.de> (raw)
In-Reply-To: <SJ0PR02MB86443029095BF5949E51808AD4FF9@SJ0PR02MB8644.namprd02.prod.outlook.com>

Hello Piyush,

On 18.08.21 12:09, Piyush Mehta wrote:
> Hi Ahmad,
> 
> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@pengutronix.de> 
> Sent: Wednesday, August 18, 2021 2:22 PM
> To: Piyush Mehta <piyushm@xilinx.com>; arnd@arndb.de; zou_wei@huawei.com; gregkh@linuxfoundation.org; linus.walleij@linaro.org; Michal Simek <michals@xilinx.com>; Jiaying Liang <jliang@xilinx.com>; iwamatsu@nigauri.org; bgolaszewski@baylibre.com; robh+dt@kernel.org; Rajan Vaja <RAJANV@xilinx.com>
> Cc: linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; git <git@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team <kernel@pengutronix.de>
> Subject: Re: [PATCH V3 3/3] gpio: modepin: Add driver support for modepin GPIO controller
> 
> On 18.08.21 10:10, Piyush Mehta wrote:
>> This patch adds driver support for the zynqmp modepin GPIO controller.
>> GPIO modepin driver set and get the value and status of the PS_MODE 
>> pin, based on device-tree pin configuration. These four mode pins are 
>> configurable as input/output. The mode pin has a control register, 
>> which have lower four-bits [0:3] are configurable as input/output, 
>> next four-bits can be used for reading the data  as input[4:7], and 
>> next setting the output pin state output[8:11].
>>
>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
>> Acked-by: Michal Simek <michal.simek@xilinx.com>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
> 
>> +/**
>> + * modepin_gpio_dir_in - Set the direction of the specified GPIO pin as input
>> + * @chip:	gpio_chip instance to be worked on
>> + * @pin:	gpio pin number within the device
>> + *
>> + * Return: 0 always
>> + */
>> +static int modepin_gpio_dir_in(struct gpio_chip *chip, unsigned int 
>> +pin) {
>> +	return 0;
>> +}
> 
> You say the gpio controller can configure pins as inputs or outputs.
> These pins are controller via firmware driver. We are updating BOOT_PIN_CTRL 	0xFF5E0250 register.
> [0:3]  = When 0, the pins will be inputs from the board to the PS. When 1, the PS will drive these pins

Ok. So if you want to configure the pin as input, you should call zynqmp_pm_bootmode_write
to write a zero into that bit.

But there's only one zynqmp_pm_bootmode_write in the GPIO driver and it's in modepin_gpio_set_value,
which does output, not input. If I understand you right, there should be a modepin_gpio_set_value in
modepin_gpio_dir_in as well?
 
> Yet, .direction_input is doing nothing. So, it's not clear to me, how this sequence could work:
> 
>  - set gpio output high (writes bootmode)
>  - set gpio to input (no-op, pin will remain high, not high impedance)

This is a valid sequence for a GPIO consumer and I don't see how this GPIO driver could
honor it. Could you clarify?

Cheers,
Ahmad

> 
> 
> 
> 
> 
> 
> I didn't check the previous discussions, but if this indeed works as intended, the how should be written here into the driver. That is a more useful comment than kernel doc for a stub function.
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2021-08-18 13:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-18  8:10 [PATCH V3 0/3] " Piyush Mehta
2021-08-18  8:10 ` [PATCH V3 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin Piyush Mehta
2021-08-18  8:10 ` [PATCH V3 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin Piyush Mehta
2021-08-18  9:00   ` Ahmad Fatoum
2021-08-18  9:38     ` Michal Simek
2021-08-18  9:55       ` Ahmad Fatoum
2021-08-18 10:01         ` Michal Simek
2021-08-23 18:09   ` Rob Herring
2021-08-18  8:10 ` [PATCH V3 3/3] gpio: modepin: Add driver support for modepin GPIO controller Piyush Mehta
2021-08-18  8:52   ` Ahmad Fatoum
2021-08-18 10:09     ` Piyush Mehta
2021-08-18 13:05       ` Ahmad Fatoum [this message]
2021-08-23  8:02   ` Bartosz Golaszewski
2021-08-23  8:14     ` Michal Simek
2021-09-22 10:18       ` Bartosz Golaszewski
2021-09-22 10:21         ` Bartosz Golaszewski
2021-09-22 10:23           ` Michal Simek

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=a1bd7d9f-5746-2d4c-8658-beb804468846@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=RAJANV@xilinx.com \
    --cc=arnd@arndb.de \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=git@xilinx.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=iwamatsu@nigauri.org \
    --cc=jliang@xilinx.com \
    --cc=kernel@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michals@xilinx.com \
    --cc=piyushm@xilinx.com \
    --cc=robh+dt@kernel.org \
    --cc=sgoud@xilinx.com \
    --cc=zou_wei@huawei.com \
    --subject='Re: [PATCH V3 3/3] gpio: modepin: Add driver support for modepin GPIO controller' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).