| Message ID | 1448446739-5541-4-git-send-email-mschiller@tdt.de |
|---|---|
| State | New |
| Headers | show |
Hi On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller <mschiller@tdt.de> wrote: > From: John Crispin <blogic@openwrt.org> > > This patch is included in the openwrt patchset for several years now and needs > 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. > 2. Fix GPIO Setup of GPIO Port3 This change looks fine. > 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. > Signed-off-by: John Crispin <blogic@openwrt.org> > Signed-off-by: Martin Schiller <mschiller@tdt.de> > --- Also please provide a changelog for your patches here. > drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > Jonas -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/25/2015 at 11:40 AM, Jonas Gorski wrote: > Hi > > On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller <mschiller@tdt.de> > wrote: > > From: John Crispin <blogic@openwrt.org> > > > > This patch is included in the openwrt patchset for several years now > and needs > > 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. > > > 2. Fix GPIO Setup of GPIO Port3 > > This change looks fine. > > > 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. But per se you are right, it would be better to split it up. > > > Signed-off-by: John Crispin <blogic@openwrt.org> > > Signed-off-by: Martin Schiller <mschiller@tdt.de> > > --- > > Also please provide a changelog for your patches here. OK. > > > drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++-- > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > > Jonas Martin
On 26/11/2015 07:40, Martin Schiller wrote: > On 11/25/2015 at 11:40 AM, Jonas Gorski wrote: >> Hi >> >> On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller <mschiller@tdt.de> >> wrote: >>> From: John Crispin <blogic@openwrt.org> >>> >>> This patch is included in the openwrt patchset for several years now >> and needs >>> 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 > >> >>> 2. Fix GPIO Setup of GPIO Port3 >> >> This change looks fine. >> >>> 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. > But per se you are right, it would be better to split it up. > >> >>> Signed-off-by: John Crispin <blogic@openwrt.org> >>> Signed-off-by: Martin Schiller <mschiller@tdt.de> >>> --- >> >> Also please provide a changelog for your patches here. > > OK. > >> >>> drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++-- >>> 1 file changed, 26 insertions(+), 2 deletions(-) >>> >> >> >> Jonas > > Martin > > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/26/2015 at 8:04 AM, John Crispin wrote: > > > On 26/11/2015 07:40, Martin Schiller wrote: > > On 11/25/2015 at 11:40 AM, Jonas Gorski wrote: > >> Hi > >> > >> On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller <mschiller@tdt.de> > >> wrote: > >>> From: John Crispin <blogic@openwrt.org> > >>> > >>> This patch is included in the openwrt patchset for several years > now > >> and needs > >>> 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: ######################################################################## > diff --git a/drivers/pinctrl/pinctrl-xway.c b/drivers/pinctrl/pinctrl- > 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 = { > .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}, ####################################################################### > > > > >> > >>> 2. Fix GPIO Setup of GPIO Port3 > >> > >> This change looks fine. > >> > >>> 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. > > But per se you are right, it would be better to split it up. > > > >> > >>> Signed-off-by: John Crispin <blogic@openwrt.org> > >>> Signed-off-by: Martin Schiller <mschiller@tdt.de> > >>> --- > >> > >> Also please provide a changelog for your patches here. > > > > OK. > > > >> > >>> drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++-- > >>> 1 file changed, 26 insertions(+), 2 deletions(-) > >>> > >> > >> > >> Jonas > > > > Martin > > > >
On 26/11/2015 08:15, Martin Schiller wrote: > On 11/26/2015 at 8:04 AM, John Crispin wrote: >> >> >> On 26/11/2015 07:40, Martin Schiller wrote: >>> On 11/25/2015 at 11:40 AM, Jonas Gorski wrote: >>>> Hi >>>> >>>> On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller <mschiller@tdt.de> >>>> wrote: >>>>> From: John Crispin <blogic@openwrt.org> >>>>> >>>>> This patch is included in the openwrt patchset for several years >> now >>>> and needs >>>>> 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: > ######################################################################## >> diff --git a/drivers/pinctrl/pinctrl-xway.c b/drivers/pinctrl/pinctrl- >> 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 = { >> .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}, > ####################################################################### > 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. 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 >> >>> >>>> >>>>> 2. Fix GPIO Setup of GPIO Port3 >>>> >>>> This change looks fine. >>>> >>>>> 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. >>> But per se you are right, it would be better to split it up. >>> >>>> >>>>> Signed-off-by: John Crispin <blogic@openwrt.org> >>>>> Signed-off-by: Martin Schiller <mschiller@tdt.de> >>>>> --- >>>> >>>> Also please provide a changelog for your patches here. >>> >>> OK. >>> >>>> >>>>> drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++-- >>>>> 1 file changed, 26 insertions(+), 2 deletions(-) >>>>> >>>> >>>> >>>> Jonas >>> >>> Martin >>> >>> > > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/26/2015 at 8:20 AM, John Crispin wrote: > > > On 26/11/2015 08:15, Martin Schiller wrote: > > On 11/26/2015 at 8:04 AM, John Crispin wrote: > >> > >> > >> On 26/11/2015 07:40, Martin Schiller wrote: > >>> On 11/25/2015 at 11:40 AM, Jonas Gorski wrote: > >>>> Hi > >>>> > >>>> On Wed, Nov 25, 2015 at 11:18 AM, Martin Schiller > <mschiller@tdt.de> > >>>> wrote: > >>>>> From: John Crispin <blogic@openwrt.org> > >>>>> > >>>>> This patch is included in the openwrt patchset for several years > >> now > >>>> and needs > >>>>> 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: > > > ####################################################################### > # > >> diff --git a/drivers/pinctrl/pinctrl-xway.c > b/drivers/pinctrl/pinctrl- > >> 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 = > { > >> .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}, > > > ####################################################################### > > > > 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 > > > >> > >>> > >>>> > >>>>> 2. Fix GPIO Setup of GPIO Port3 > >>>> > >>>> This change looks fine. > >>>> > >>>>> 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. > >>> But per se you are right, it would be better to split it up. > >>> > >>>> > >>>>> Signed-off-by: John Crispin <blogic@openwrt.org> > >>>>> Signed-off-by: Martin Schiller <mschiller@tdt.de> > >>>>> --- > >>>> > >>>> Also please provide a changelog for your patches here. > >>> > >>> OK. > >>> > >>>> > >>>>> drivers/pinctrl/pinctrl-xway.c | 28 ++++++++++++++++++++++++++-- > >>>>> 1 file changed, 26 insertions(+), 2 deletions(-) > >>>>> > >>>> > >>>> > >>>> Jonas > >>> > >>> Martin > >>> > >>> > > > >
diff --git a/drivers/pinctrl/pinctrl-xway.c b/drivers/pinctrl/pinctrl-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 = { .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}, @@ -1563,12 +1570,28 @@ static int xway_gpio_dir_out(struct gpio_chip *chip, unsigned int pin, int val) { struct ltq_pinmux_info *info = dev_get_drvdata(chip->dev); + if (PORT(pin) == PORT3) + gpio_setbit(info->membase[0], GPIO3_OD, PORT_PIN(pin)); + else + gpio_setbit(info->membase[0], GPIO_OD(pin), PORT_PIN(pin)); gpio_setbit(info->membase[0], GPIO_DIR(pin), PORT_PIN(pin)); xway_gpio_set(chip, pin, val); return 0; } +static int xway_gpio_to_irq(struct gpio_chip *chip, unsigned offset) +{ + struct ltq_pinmux_info *info = dev_get_drvdata(chip->dev); + int i; + + for (i = 0; i < info->num_exin; i++) + if (info->exin[i] == offset) + return ltq_eiu_get_irq(i); + + return -1; +} + static struct gpio_chip xway_chip = { .label = "gpio-xway", .direction_input = xway_gpio_dir_in, @@ -1577,6 +1600,7 @@ static struct gpio_chip xway_chip = { .set = xway_gpio_set, .request = gpiochip_generic_request, .free = gpiochip_generic_free, + .to_irq = xway_gpio_to_irq, .base = -1, };