All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Andrew Bresticker
	<abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Thierry Reding
	<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH v2 2/4] pinctrl: Add NVIDIA Tegra XUSB pad controller support
Date: Wed, 11 Jun 2014 15:06:37 -0600	[thread overview]
Message-ID: <5398C4DD.6020509@wwwdotorg.org> (raw)
In-Reply-To: <CAL1qeaGKUGjSzriDGptR+h23+bS3ZWuStz6koPURCstkFbEjNA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 06/11/2014 02:23 PM, Andrew Bresticker wrote:
> On Tue, Jun 10, 2014 at 4:11 AM, Thierry Reding
> <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> The XUSB pad controller found on NVIDIA Tegra SoCs provides several pads
>> that lanes can be assigned to in order to support a variety of interface
>> options: USB 2.0, USB 3.0, PCIe and SATA.
>>
>> In addition to the pin controller used to assign lanes to pads two PHYs
>> are exposed to allow the bricks for PCIe and SATA to be powered up and
>> down by PCIe and SATA drivers.

>> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c

>> +static int tegra_xusb_padctl_pinconf_group_set(struct pinctrl_dev *pinctrl,
>> +                                              unsigned int group,
>> +                                              unsigned long *configs,
>> +                                              unsigned int num_configs)

>> +       for (i = 0; i < num_configs; i++) {
>> +               param = TEGRA_XUSB_PADCTL_UNPACK_PARAM(configs[i]);
>> +               value = TEGRA_XUSB_PADCTL_UNPACK_VALUE(configs[i]);
>> +
>> +               switch (param) {
>> +               case TEGRA_XUSB_PADCTL_IDDQ:
>> +                       value = padctl_readl(padctl, lane->offset);
> 
> This overwrites the configuration value - probably want to use a
> separate variable for the register value.

It'd be nice to trim what you quote so that people don't have to wade
through hundreds of lines of code to find a 2-line comment. It's easily
missed.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Andrew Bresticker <abrestic@chromium.org>,
	Thierry Reding <thierry.reding@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	devicetree@vger.kernel.org,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/4] pinctrl: Add NVIDIA Tegra XUSB pad controller support
Date: Wed, 11 Jun 2014 15:06:37 -0600	[thread overview]
Message-ID: <5398C4DD.6020509@wwwdotorg.org> (raw)
In-Reply-To: <CAL1qeaGKUGjSzriDGptR+h23+bS3ZWuStz6koPURCstkFbEjNA@mail.gmail.com>

On 06/11/2014 02:23 PM, Andrew Bresticker wrote:
> On Tue, Jun 10, 2014 at 4:11 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> The XUSB pad controller found on NVIDIA Tegra SoCs provides several pads
>> that lanes can be assigned to in order to support a variety of interface
>> options: USB 2.0, USB 3.0, PCIe and SATA.
>>
>> In addition to the pin controller used to assign lanes to pads two PHYs
>> are exposed to allow the bricks for PCIe and SATA to be powered up and
>> down by PCIe and SATA drivers.

>> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c

>> +static int tegra_xusb_padctl_pinconf_group_set(struct pinctrl_dev *pinctrl,
>> +                                              unsigned int group,
>> +                                              unsigned long *configs,
>> +                                              unsigned int num_configs)

>> +       for (i = 0; i < num_configs; i++) {
>> +               param = TEGRA_XUSB_PADCTL_UNPACK_PARAM(configs[i]);
>> +               value = TEGRA_XUSB_PADCTL_UNPACK_VALUE(configs[i]);
>> +
>> +               switch (param) {
>> +               case TEGRA_XUSB_PADCTL_IDDQ:
>> +                       value = padctl_readl(padctl, lane->offset);
> 
> This overwrites the configuration value - probably want to use a
> separate variable for the register value.

It'd be nice to trim what you quote so that people don't have to wade
through hundreds of lines of code to find a 2-line comment. It's easily
missed.

WARNING: multiple messages have this Message-ID (diff)
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/4] pinctrl: Add NVIDIA Tegra XUSB pad controller support
Date: Wed, 11 Jun 2014 15:06:37 -0600	[thread overview]
Message-ID: <5398C4DD.6020509@wwwdotorg.org> (raw)
In-Reply-To: <CAL1qeaGKUGjSzriDGptR+h23+bS3ZWuStz6koPURCstkFbEjNA@mail.gmail.com>

On 06/11/2014 02:23 PM, Andrew Bresticker wrote:
> On Tue, Jun 10, 2014 at 4:11 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> The XUSB pad controller found on NVIDIA Tegra SoCs provides several pads
>> that lanes can be assigned to in order to support a variety of interface
>> options: USB 2.0, USB 3.0, PCIe and SATA.
>>
>> In addition to the pin controller used to assign lanes to pads two PHYs
>> are exposed to allow the bricks for PCIe and SATA to be powered up and
>> down by PCIe and SATA drivers.

>> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c

>> +static int tegra_xusb_padctl_pinconf_group_set(struct pinctrl_dev *pinctrl,
>> +                                              unsigned int group,
>> +                                              unsigned long *configs,
>> +                                              unsigned int num_configs)

>> +       for (i = 0; i < num_configs; i++) {
>> +               param = TEGRA_XUSB_PADCTL_UNPACK_PARAM(configs[i]);
>> +               value = TEGRA_XUSB_PADCTL_UNPACK_VALUE(configs[i]);
>> +
>> +               switch (param) {
>> +               case TEGRA_XUSB_PADCTL_IDDQ:
>> +                       value = padctl_readl(padctl, lane->offset);
> 
> This overwrites the configuration value - probably want to use a
> separate variable for the register value.

It'd be nice to trim what you quote so that people don't have to wade
through hundreds of lines of code to find a 2-line comment. It's easily
missed.

  parent reply	other threads:[~2014-06-11 21:06 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-10 11:11 [PATCH v2 1/4] of: Add NVIDIA Tegra XUSB pad controller binding Thierry Reding
2014-06-10 11:11 ` Thierry Reding
2014-06-10 11:11 ` Thierry Reding
     [not found] ` <1402398708-10722-1-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-10 11:11   ` [PATCH v2 2/4] pinctrl: Add NVIDIA Tegra XUSB pad controller support Thierry Reding
2014-06-10 11:11     ` Thierry Reding
2014-06-10 11:11     ` Thierry Reding
2014-06-11 20:23     ` Andrew Bresticker
2014-06-11 20:23       ` Andrew Bresticker
2014-06-11 20:23       ` Andrew Bresticker
     [not found]       ` <CAL1qeaGKUGjSzriDGptR+h23+bS3ZWuStz6koPURCstkFbEjNA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-06-11 21:06         ` Stephen Warren [this message]
2014-06-11 21:06           ` Stephen Warren
2014-06-11 21:06           ` Stephen Warren
2014-06-12  7:22       ` Thierry Reding
2014-06-12  7:22         ` Thierry Reding
2014-06-12  7:22         ` Thierry Reding
     [not found]     ` <1402398708-10722-2-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-12 20:39       ` Stephen Warren
2014-06-12 20:39         ` Stephen Warren
2014-06-12 20:39         ` Stephen Warren
2014-07-04 23:04         ` Linus Walleij
2014-07-04 23:04           ` Linus Walleij
2014-07-04 23:04           ` Linus Walleij
2014-07-04 23:01   ` [PATCH v2 1/4] of: Add NVIDIA Tegra XUSB pad controller binding Linus Walleij
2014-07-04 23:01     ` Linus Walleij
2014-07-04 23:01     ` Linus Walleij
2014-06-10 11:11 ` [PATCH v2 3/4] ARM: tegra: tegra124: Add XUSB pad controller Thierry Reding
2014-06-10 11:11   ` Thierry Reding
2014-06-10 11:11 ` [PATCH v2 4/4] ARM: tegra: jetson-tk1: " Thierry Reding
2014-06-10 11:11   ` Thierry Reding

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=5398C4DD.6020509@wwwdotorg.org \
    --to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
    --cc=abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.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.