All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 7/7] pinctrl: qcom-spmi-gpio: Migrate to pinconf-generic
Date: Fri, 05 Dec 2014 17:37:03 +0000	[thread overview]
Message-ID: <40e6a2a0c9f54ba4805871cf33373561@BN1BFFO11FD003.protection.gbl> (raw)
In-Reply-To: <bc08b9044a3f46f5a575dca2fa18a663@BY2FFO11FD057.protection.gbl>

On Fri, 2014-12-05 at 09:08AM -0800, Sören Brinkmann wrote:
> On Fri, 2014-12-05 at 09:59AM +0200, Ivan T. Ivanov wrote:
> > 
> > On Thu, 2014-11-27 at 17:26 -0800, Soren Brinkmann wrote:
> > > 
> > >  static const struct pinconf_ops pmic_gpio_pinconf_ops = {
> > > +       .is_generic= true,
> > >         .pin_config_group_get= pmic_gpio_config_get,
> > >         .pin_config_group_set= pmic_gpio_config_set,
> > >         .pin_config_group_dbg_show= pmic_gpio_config_dbg_show,
> > > @@ -848,6 +742,9 @@ static int pmic_gpio_probe(struct platform_device *pdev)
> > >         pctrldesc->name = dev_name(dev);
> > >         pctrldesc->pins = pindesc;
> > >         pctrldesc->npins = npins;
> > > +       pctrldesc->num_dt_params = ARRAY_SIZE(pmic_gpio_bindings);
> > > +       pctrldesc->params = pmic_gpio_bindings;
> > > +       pctrldesc->conf_items = pmic_conf_items;
> > > 
> > 
> > What will happen if number of conf_items is less than number of params?
> 
> I suppose bad things :) You're right, that should probably be checked
> somewhere. Let me see whether I find a good place to put such a check.

Looking at it, it's not that straight forward. We only pass one size to
the core, so the core cannot really check that the number of entries in
both arrays are equal. And passing the size of both looks a little
redundant to me.
That would put the driver in charge of making sure these arrays are
aligned.

With the change given below, it would at least trigger a compiler
warning. I'll add that to the next iteration of this series.
Longer term, merging those arrays, as you suggested, may be a better
solution.

	Thanks,
	Sören

-------------8<-------------8<-------------------8<------------------------8<---
diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index 29b75134771f..5942af4c2e67 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -924,7 +924,7 @@ static const struct pinconf_generic_dt_params zynq_dt_params[] = {
        { "io-standard", PIN_CONFIG_IOSTANDARD, zynq_iostd_lvcmos18},
 };
 
-static const struct pin_config_item zynq_conf_items[] = {
+static const struct pin_config_item zynq_conf_items[ARRAY_SIZE(zynq_dt_params)] = {
        PCONFDUMP(PIN_CONFIG_IOSTANDARD, "IO-standard", NULL, true),
 };
 
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index c612aaa653d5..17f811c9c2c0 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -136,7 +136,7 @@ static const struct pinconf_generic_dt_params pmic_gpio_bindings[] = {
        {"qcom,drive-strength",         PMIC_GPIO_CONF_STRENGTH,        0},
 };
 
-static const struct pin_config_item pmic_conf_items[] = {
+static const struct pin_config_item pmic_conf_items[ARRAY_SIZE(pmic_gpio_bindings)] = {
        PCONFDUMP(PMIC_GPIO_CONF_PULL_UP,  "pull up strength", NULL, true),
        PCONFDUMP(PMIC_GPIO_CONF_STRENGTH, "drive-strength", NULL, true),
 };

WARNING: multiple messages have this Message-ID
From: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
To: "Ivan T. Ivanov" <iivanov@mm-sol.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Michal Simek <michal.simek@xilinx.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Alessandro Rubini <rubini@unipv.it>,
	Heiko Stuebner <heiko@sntech.de>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	<linux-rockchip@lists.infradead.org>, <linux-sh@vger.kernel.org>,
	Bjorn Andersson <bjorn.andersson@sonymobile.com>,
	Beniamino Galvani <b.galvani@gmail.com>
Subject: Re: [PATCH v2 7/7] pinctrl: qcom-spmi-gpio: Migrate to pinconf-generic
Date: Fri, 5 Dec 2014 09:37:03 -0800	[thread overview]
Message-ID: <40e6a2a0c9f54ba4805871cf33373561@BN1BFFO11FD003.protection.gbl> (raw)
In-Reply-To: <bc08b9044a3f46f5a575dca2fa18a663@BY2FFO11FD057.protection.gbl>

On Fri, 2014-12-05 at 09:08AM -0800, Sören Brinkmann wrote:
> On Fri, 2014-12-05 at 09:59AM +0200, Ivan T. Ivanov wrote:
> > 
> > On Thu, 2014-11-27 at 17:26 -0800, Soren Brinkmann wrote:
> > > 
> > >  static const struct pinconf_ops pmic_gpio_pinconf_ops = {
> > > +       .is_generic= true,
> > >         .pin_config_group_get= pmic_gpio_config_get,
> > >         .pin_config_group_set= pmic_gpio_config_set,
> > >         .pin_config_group_dbg_show= pmic_gpio_config_dbg_show,
> > > @@ -848,6 +742,9 @@ static int pmic_gpio_probe(struct platform_device *pdev)
> > >         pctrldesc->name = dev_name(dev);
> > >         pctrldesc->pins = pindesc;
> > >         pctrldesc->npins = npins;
> > > +       pctrldesc->num_dt_params = ARRAY_SIZE(pmic_gpio_bindings);
> > > +       pctrldesc->params = pmic_gpio_bindings;
> > > +       pctrldesc->conf_items = pmic_conf_items;
> > > 
> > 
> > What will happen if number of conf_items is less than number of params?
> 
> I suppose bad things :) You're right, that should probably be checked
> somewhere. Let me see whether I find a good place to put such a check.

Looking at it, it's not that straight forward. We only pass one size to
the core, so the core cannot really check that the number of entries in
both arrays are equal. And passing the size of both looks a little
redundant to me.
That would put the driver in charge of making sure these arrays are
aligned.

With the change given below, it would at least trigger a compiler
warning. I'll add that to the next iteration of this series.
Longer term, merging those arrays, as you suggested, may be a better
solution.

	Thanks,
	Sören

-------------8<-------------8<-------------------8<------------------------8<---
diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index 29b75134771f..5942af4c2e67 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -924,7 +924,7 @@ static const struct pinconf_generic_dt_params zynq_dt_params[] = {
        { "io-standard", PIN_CONFIG_IOSTANDARD, zynq_iostd_lvcmos18},
 };
 
-static const struct pin_config_item zynq_conf_items[] = {
+static const struct pin_config_item zynq_conf_items[ARRAY_SIZE(zynq_dt_params)] = {
        PCONFDUMP(PIN_CONFIG_IOSTANDARD, "IO-standard", NULL, true),
 };
 
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index c612aaa653d5..17f811c9c2c0 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -136,7 +136,7 @@ static const struct pinconf_generic_dt_params pmic_gpio_bindings[] = {
        {"qcom,drive-strength",         PMIC_GPIO_CONF_STRENGTH,        0},
 };
 
-static const struct pin_config_item pmic_conf_items[] = {
+static const struct pin_config_item pmic_conf_items[ARRAY_SIZE(pmic_gpio_bindings)] = {
        PCONFDUMP(PMIC_GPIO_CONF_PULL_UP,  "pull up strength", NULL, true),
        PCONFDUMP(PMIC_GPIO_CONF_STRENGTH, "drive-strength", NULL, true),
 };

WARNING: multiple messages have this Message-ID
From: soren.brinkmann@xilinx.com (Sören Brinkmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 7/7] pinctrl: qcom-spmi-gpio: Migrate to pinconf-generic
Date: Fri, 5 Dec 2014 09:37:03 -0800	[thread overview]
Message-ID: <40e6a2a0c9f54ba4805871cf33373561@BN1BFFO11FD003.protection.gbl> (raw)
In-Reply-To: <bc08b9044a3f46f5a575dca2fa18a663@BY2FFO11FD057.protection.gbl>

On Fri, 2014-12-05 at 09:08AM -0800, S?ren Brinkmann wrote:
> On Fri, 2014-12-05 at 09:59AM +0200, Ivan T. Ivanov wrote:
> > 
> > On Thu, 2014-11-27 at 17:26 -0800, Soren Brinkmann wrote:
> > > 
> > >  static const struct pinconf_ops pmic_gpio_pinconf_ops = {
> > > +       .is_generic= true,
> > >         .pin_config_group_get= pmic_gpio_config_get,
> > >         .pin_config_group_set= pmic_gpio_config_set,
> > >         .pin_config_group_dbg_show= pmic_gpio_config_dbg_show,
> > > @@ -848,6 +742,9 @@ static int pmic_gpio_probe(struct platform_device *pdev)
> > >         pctrldesc->name = dev_name(dev);
> > >         pctrldesc->pins = pindesc;
> > >         pctrldesc->npins = npins;
> > > +       pctrldesc->num_dt_params = ARRAY_SIZE(pmic_gpio_bindings);
> > > +       pctrldesc->params = pmic_gpio_bindings;
> > > +       pctrldesc->conf_items = pmic_conf_items;
> > > 
> > 
> > What will happen if number of conf_items is less than number of params?
> 
> I suppose bad things :) You're right, that should probably be checked
> somewhere. Let me see whether I find a good place to put such a check.

Looking at it, it's not that straight forward. We only pass one size to
the core, so the core cannot really check that the number of entries in
both arrays are equal. And passing the size of both looks a little
redundant to me.
That would put the driver in charge of making sure these arrays are
aligned.

With the change given below, it would at least trigger a compiler
warning. I'll add that to the next iteration of this series.
Longer term, merging those arrays, as you suggested, may be a better
solution.

	Thanks,
	S?ren

-------------8<-------------8<-------------------8<------------------------8<---
diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index 29b75134771f..5942af4c2e67 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -924,7 +924,7 @@ static const struct pinconf_generic_dt_params zynq_dt_params[] = {
        { "io-standard", PIN_CONFIG_IOSTANDARD, zynq_iostd_lvcmos18},
 };
 
-static const struct pin_config_item zynq_conf_items[] = {
+static const struct pin_config_item zynq_conf_items[ARRAY_SIZE(zynq_dt_params)] = {
        PCONFDUMP(PIN_CONFIG_IOSTANDARD, "IO-standard", NULL, true),
 };
 
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index c612aaa653d5..17f811c9c2c0 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -136,7 +136,7 @@ static const struct pinconf_generic_dt_params pmic_gpio_bindings[] = {
        {"qcom,drive-strength",         PMIC_GPIO_CONF_STRENGTH,        0},
 };
 
-static const struct pin_config_item pmic_conf_items[] = {
+static const struct pin_config_item pmic_conf_items[ARRAY_SIZE(pmic_gpio_bindings)] = {
        PCONFDUMP(PMIC_GPIO_CONF_PULL_UP,  "pull up strength", NULL, true),
        PCONFDUMP(PMIC_GPIO_CONF_STRENGTH, "drive-strength", NULL, true),
 };

  reply	other threads:[~2014-12-05 17:37 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-28  1:26 [PATCH v2 0/7] Pinctrl support for Zynq Soren Brinkmann
2014-11-28  1:26 ` [PATCH v2 1/7] pinctrl: pinconf-generic: Infer map type from DT property Soren Brinkmann
2014-12-02 15:01   ` Linus Walleij
2014-12-03 23:04     ` Sören Brinkmann
2014-11-28  1:26 ` [PATCH v2 2/7] pinctrl: pinconf-generic: Allow driver to specify DT params Soren Brinkmann
2014-11-28  1:26 ` [PATCH v2 3/7] pinctrl: zynq: Document DT binding Soren Brinkmann
2014-11-28  1:26 ` [PATCH v2 4/7] pinctrl: Add driver for Zynq Soren Brinkmann
2014-11-28  1:26 ` [PATCH v2 5/7] ARM: zynq: Enable pinctrl Soren Brinkmann
2014-11-28  1:26 ` [PATCH v2 6/7] ARM: zynq: DT: Add pinctrl information Soren Brinkmann
2014-11-28  1:26 ` [PATCH v2 7/7] pinctrl: qcom-spmi-gpio: Migrate to pinconf-generic Soren Brinkmann
2014-12-03 13:03   ` Ivan T. Ivanov
2014-12-03 17:38     ` Sören Brinkmann
2014-12-04  9:30       ` Ivan T. Ivanov
2014-12-04 17:21         ` Sören Brinkmann
2014-12-05  7:59   ` Ivan T. Ivanov
2014-12-05 17:08     ` Sören Brinkmann
2014-12-05 17:37       ` Sören Brinkmann [this message]
2014-12-12 16:21       ` Sören Brinkmann
2014-12-03 12:51 ` [PATCH v2 0/7] Pinctrl support for Zynq Ivan T. Ivanov
2014-12-03 17:59   ` Sören Brinkmann
2014-12-04  9:44 ` Linus Walleij
2014-12-04 17:28   ` Sören Brinkmann

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=40e6a2a0c9f54ba4805871cf33373561@BN1BFFO11FD003.protection.gbl \
    --to=soren.brinkmann@xilinx.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --subject='Re: [PATCH v2 7/7] pinctrl: qcom-spmi-gpio: Migrate to pinconf-generic' \
    /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 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.