Re: [PATCH v2 4/4] pinctrl/lantiq: fix up pinmux

[John Crispin <blogic@openwrt.org>]

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
>>>
>>>
> 
>