On 11/26/2015 at 8:20 AM, John Crispin wrote:

On 26/11/2015 08:15, Martin Schiller wrote:

quoted

On 11/26/2015 at 8:04 AM, John Crispin wrote:

quoted

On 26/11/2015 07:40, Martin Schiller wrote:

quoted

On 11/25/2015 at 11:40 AM, Jonas Gorski wrote:

quoted

Hi

On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller

[off-list ref]

quoted

quoted

quoted

quoted

wrote:

quoted

From: John Crispin <redacted>

This patch is included in the openwrt patchset for several years

now

quoted

quoted

and needs

quoted

to go upstream as well. It includes the following changes:
1. Fix up inline function call to xway_mux_apply

This really needs an explanation what is being fixed here.

I hope John - as the original author of this patch - can explain
why this change is necessary.

what change? why am I in Cc: and not To: if an action is required ?

John

That change is meant:

#######################################################################
#

quoted

quoted

diff --git a/drivers/pinctrl/pinctrl-xway.c

b/drivers/pinctrl/pinctrl-

quoted

quoted

xway.c
index a064962..f0b1b48 100644

--- a/drivers/pinctrl/pinctrl-xway.c
+++ b/drivers/pinctrl/pinctrl-xway.c

@@ -1496,10 +1496,9 @@ static struct pinctrl_desc xway_pctrl_desc =

{

quoted

quoted

 .confops= &xway_pinconf_ops,
 };

-static inline int xway_mux_apply(struct pinctrl_dev *pctrldev,
+static int mux_apply(struct ltq_pinmux_info *info,
 int pin, int mux)
 {
-struct ltq_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
 int port = PORT(pin);
 u32 alt1_reg = GPIO_ALT1(pin);

@@ -1519,6 +1518,14 @@ static inline int xway_mux_apply(struct

pinctrl_dev *pctrldev,
 return 0;
 }

+static inline int xway_mux_apply(struct pinctrl_dev *pctrldev,
+int pin, int mux)
+{
+struct ltq_pinmux_info *info = pinctrl_dev_get_drvdata(pctrldev);
+
+return mux_apply(info, pin, mux);
+}
+
 static const struct ltq_cfg_param xway_cfg_params[] = {
 {"lantiq,pull",LTQ_PINCONF_PARAM_PULL},
 {"lantiq,open-drain",LTQ_PINCONF_PARAM_OPEN_DRAIN},

#######################################################################

quoted

ok so you picked up a patch and sent it upstream without looking at
what
it really does. the patch is simply not ready for upstream. the problem
here is copy & paste inconsistency.

Of course I analyzed the whole patch and - as you may have noticed - added
a description for the 3 changes which were made in this patch.

I also know that this first part of the patch changes the scope of the inline
code, but I did not know why this change was done.

however if we want to resolve this we should either keep the inlines
and
just stick to the current code pattern used or we could just assume
that
the compiler will be smart enough to to know when to inline and remove
all of them.

i'll leave it up to you to decide and propose your solution as a patch
with an explanation.

John

So I think it would be the best for now to leave this code as it is and make
two separate patches from the remaining two changes:

- Fix GPIO Setup of GPIO Port3
- Implement gpio_chip.to_irq

Martin

quoted

quoted

quoted

quoted

quoted

2. Fix GPIO Setup of GPIO Port3

This change looks fine.

quoted

3. Implement gpio_chip.to_irq

These are three different changes (two fixes, one new feature) and
therefore should be split up into three patches.

As I'm not the author of this patch, I decided to leave it as it

is.

quoted

quoted

quoted

But per se you are right, it would be better to split it up.

quoted

quoted

Signed-off-by: John Crispin <redacted>
Signed-off-by: Martin Schiller <redacted>
---

Also please provide a changelog for your patches here.

OK.

quoted

quoted

 drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

Jonas

Martin