All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>,
	Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 4/5] PCI: mvebu: add support for reset on GPIO
Date: Tue, 13 Aug 2013 22:29:43 +0200	[thread overview]
Message-ID: <20130813222943.46fdacff@skate> (raw)
In-Reply-To: <1376396724-32048-5-git-send-email-sebastian.hesselbarth@gmail.com>

Dear Sebastian Hesselbarth,

On Tue, 13 Aug 2013 14:25:23 +0200, Sebastian Hesselbarth wrote:

> +		port->reset_gpio = of_get_named_gpio_flags(child,
> +						   "reset-gpios", 0, &flags);
> +		if (gpio_is_valid(port->reset_gpio)) {
> +			u32 reset_udelay = 20000;
> +
> +			port->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
> +			port->reset_name = kasprintf(GFP_KERNEL,
> +				     "pcie%d.%d-reset", port->port, port->lane);
> +			of_property_read_u32(child, "reset-delay-us",
> +					     &reset_udelay);
> +
> +			ret = devm_gpio_request_one(&pdev->dev,
> +			    port->reset_gpio, GPIOF_DIR_OUT, port->reset_name);
> +			if (ret) {
> +				if (ret == -EPROBE_DEFER)
> +					return ret;
> +				continue;
> +			}
> +
> +			gpio_set_value(port->reset_gpio,
> +				       (port->reset_active_low) ? 1 : 0);
> +			udelay(reset_udelay);
> +		}

Sorry for raising this only now, but I think I would have preferred to
see this reset-gpios handling be moved into a separate sub-function.
The loop initializing each PCIe interface is already quite large, and I
believe moving this reset-gpios thing to a sub-function would have made
sense.

But well, the patches have been applied, and we can always adjust this
with a followup patch.

Jason, have you re-created your for-next branch with all those patches?
I'd like to give them a test if possible.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/5] PCI: mvebu: add support for reset on GPIO
Date: Tue, 13 Aug 2013 22:29:43 +0200	[thread overview]
Message-ID: <20130813222943.46fdacff@skate> (raw)
In-Reply-To: <1376396724-32048-5-git-send-email-sebastian.hesselbarth@gmail.com>

Dear Sebastian Hesselbarth,

On Tue, 13 Aug 2013 14:25:23 +0200, Sebastian Hesselbarth wrote:

> +		port->reset_gpio = of_get_named_gpio_flags(child,
> +						   "reset-gpios", 0, &flags);
> +		if (gpio_is_valid(port->reset_gpio)) {
> +			u32 reset_udelay = 20000;
> +
> +			port->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
> +			port->reset_name = kasprintf(GFP_KERNEL,
> +				     "pcie%d.%d-reset", port->port, port->lane);
> +			of_property_read_u32(child, "reset-delay-us",
> +					     &reset_udelay);
> +
> +			ret = devm_gpio_request_one(&pdev->dev,
> +			    port->reset_gpio, GPIOF_DIR_OUT, port->reset_name);
> +			if (ret) {
> +				if (ret == -EPROBE_DEFER)
> +					return ret;
> +				continue;
> +			}
> +
> +			gpio_set_value(port->reset_gpio,
> +				       (port->reset_active_low) ? 1 : 0);
> +			udelay(reset_udelay);
> +		}

Sorry for raising this only now, but I think I would have preferred to
see this reset-gpios handling be moved into a separate sub-function.
The loop initializing each PCIe interface is already quite large, and I
believe moving this reset-gpios thing to a sub-function would have made
sense.

But well, the patches have been applied, and we can always adjust this
with a followup patch.

Jason, have you re-created your for-next branch with all those patches?
I'd like to give them a test if possible.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

  reply	other threads:[~2013-08-13 20:29 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-13 12:25 [PATCH v2 0/5] ARM: dove: DT PCIe support Sebastian Hesselbarth
2013-08-13 12:25 ` Sebastian Hesselbarth
2013-08-13 12:25 ` [PATCH v2 1/5] PCI: mvebu: move clock enable before register access Sebastian Hesselbarth
2013-08-13 12:25   ` Sebastian Hesselbarth
2013-08-13 12:25 ` [PATCH v2 2/5] PCI: mvebu: increment nports only for registered ports Sebastian Hesselbarth
2013-08-13 12:25   ` Sebastian Hesselbarth
2013-08-13 12:25 ` [PATCH v2 3/5] PCI: mvebu: remove subsys_initcall Sebastian Hesselbarth
2013-08-13 12:25   ` Sebastian Hesselbarth
2013-08-13 12:25 ` [PATCH v2 4/5] PCI: mvebu: add support for reset on GPIO Sebastian Hesselbarth
2013-08-13 12:25   ` Sebastian Hesselbarth
2013-08-13 20:29   ` Thomas Petazzoni [this message]
2013-08-13 20:29     ` Thomas Petazzoni
2013-08-13 21:26     ` Jason Cooper
2013-08-13 21:26       ` Jason Cooper
2013-08-14  9:07   ` Thierry Reding
2013-08-14  9:07     ` Thierry Reding
2013-08-14  9:25     ` Sebastian Hesselbarth
2013-08-14  9:25       ` Sebastian Hesselbarth
2013-08-14 11:53       ` Jason Cooper
2013-08-14 11:53         ` Jason Cooper
2013-08-13 12:25 ` [PATCH v2 5/5] PCI: mvebu: add support for Marvell Dove SoCs Sebastian Hesselbarth
2013-08-13 12:25   ` Sebastian Hesselbarth
2013-08-13 14:01 ` [PATCH v2 0/5] ARM: dove: DT PCIe support Jason Cooper
2013-08-13 14:01   ` Jason Cooper
2013-08-13 15:11   ` Jason Cooper
2013-08-13 15:11     ` Jason Cooper
2013-08-13 17:19   ` Bjorn Helgaas
2013-08-13 17:19     ` Bjorn Helgaas
2013-08-13 18:49 ` Jason Cooper
2013-08-13 18:49   ` Jason Cooper
2013-08-15 16:16 ` Thomas Petazzoni
2013-08-15 16:16   ` Thomas Petazzoni

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=20130813222943.46fdacff@skate \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=andrew@lunn.ch \
    --cc=bhelgaas@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@gmail.com \
    /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.