2014-01-16 13:19:14

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On Sat, Dec 21, 2013 at 3:13 AM, Sherman Yin <[email protected]> wrote:

'> Adds pinctrl driver for Broadcom Capri (BCM281xx) SoCs.
>
> Signed-off-by: Sherman Yin <[email protected]>
> Reviewed-by: Christian Daudt <[email protected]>
> Reviewed-by: Matt Porter <[email protected]>
> ---
> v4: - PINCTRL selected in Kconfig, PINCTRL_CAPRI selected in bcm_defconfig
> - make use of regmap
> - change CAPRI_PIN_UPDATE from macro to inline function.
> - Handle pull-up strength arg in Ohm instead of enum

Patch applied. It is really good now! It's late before the merge
window, but you've done a tremendous work on this driver and
I don't want to delay its deployment further.

But note:

> arch/arm/configs/bcm_defconfig | 1 +
> arch/arm/mach-bcm/Kconfig | 1 +

I've optimistically applied these two hunks of the patch as well,
but if there is any conflict with the ARM SoC tree I will just rebase
the patch and pull these *out*.

ARM SoC maintainers: be warned if something collides.

Yours,
Linus Walleij


2014-01-17 20:00:00

by Sherman Yin

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On 14-01-16 05:19 AM, Linus Walleij wrote:
> On Sat, Dec 21, 2013 at 3:13 AM, Sherman Yin <[email protected]> wrote:
>
> '> Adds pinctrl driver for Broadcom Capri (BCM281xx) SoCs.
>>
>> Signed-off-by: Sherman Yin <[email protected]>
>> Reviewed-by: Christian Daudt <[email protected]>
>> Reviewed-by: Matt Porter <[email protected]>
>> ---
>> v4: - PINCTRL selected in Kconfig, PINCTRL_CAPRI selected in bcm_defconfig
>> - make use of regmap
>> - change CAPRI_PIN_UPDATE from macro to inline function.
>> - Handle pull-up strength arg in Ohm instead of enum
>
> Patch applied. It is really good now! It's late before the merge
> window, but you've done a tremendous work on this driver and
> I don't want to delay its deployment further.

Great, thanks for the support and reviews!

> But note:
>
>> arch/arm/configs/bcm_defconfig | 1 +
>> arch/arm/mach-bcm/Kconfig | 1 +
>
> I've optimistically applied these two hunks of the patch as well,
> but if there is any conflict with the ARM SoC tree I will just rebase
> the patch and pull these *out*.
>
> ARM SoC maintainers: be warned if something collides.

Ok, will keep an eye on this and fix accordingly. Should be simple fixes.

Regards,
Sherman

2014-01-18 02:56:11

by Matt Porter

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On Fri, Jan 17, 2014 at 11:59:21AM -0800, Sherman Yin wrote:
> On 14-01-16 05:19 AM, Linus Walleij wrote:
> >On Sat, Dec 21, 2013 at 3:13 AM, Sherman Yin <[email protected]> wrote:
> >
> >'> Adds pinctrl driver for Broadcom Capri (BCM281xx) SoCs.
> >>
> >>Signed-off-by: Sherman Yin <[email protected]>
> >>Reviewed-by: Christian Daudt <[email protected]>
> >>Reviewed-by: Matt Porter <[email protected]>
> >>---
> >>v4: - PINCTRL selected in Kconfig, PINCTRL_CAPRI selected in bcm_defconfig
> >> - make use of regmap
> >> - change CAPRI_PIN_UPDATE from macro to inline function.
> >> - Handle pull-up strength arg in Ohm instead of enum
> >
> >Patch applied. It is really good now! It's late before the merge
> >window, but you've done a tremendous work on this driver and
> >I don't want to delay its deployment further.
>
> Great, thanks for the support and reviews!

Very nice! Now after having completely missing something fundamental on
my reviews, I feel compelled to bring it up at the 11^H^H12th hour.

That is, this is the *only* BCM281xx driver to be named Capri, both in
the filename and driver code, but also in the binding compatible. We
didn't do that on anything else that's gone upstream to date. This
really introduces an unfortunate inconsistency as it obscures which
SoC family this binding and driver belong with.

I wonder if Linus would accept a rename at this point (too late for 3.14
presumably, but for 3.15) of s/capri/bcm281xx throughout, bcm11351 for
the compatible string, as we have for the machine compatible, and also
BCM281XX for the Kconfig option. If not, I'll survive, but it pains me
to see one thing completely different out of this entire family. If
nothing else, it would be great to address the compatible string before
this hits the 3.14 release.

Linus?

-Matt

2014-01-20 08:16:28

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On Sat, Jan 18, 2014 at 3:56 AM, Matt Porter <[email protected]> wrote:

> I wonder if Linus would accept a rename at this point (too late for 3.14
> presumably, but for 3.15) of s/capri/bcm281xx throughout, bcm11351 for
> the compatible string, as we have for the machine compatible, and also
> BCM281XX for the Kconfig option.

Yes, if there is some consensus that this is what we want to do.

I can certainly merge that during the 3.14-rc phase for that matter.

Yours,
Linus Walleij

2014-01-20 19:15:37

by Sherman Yin

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On 14-01-20 12:16 AM, Linus Walleij wrote:
> On Sat, Jan 18, 2014 at 3:56 AM, Matt Porter <[email protected]> wrote:
>
>> I wonder if Linus would accept a rename at this point (too late for 3.14
>> presumably, but for 3.15) of s/capri/bcm281xx throughout, bcm11351 for
>> the compatible string, as we have for the machine compatible, and also
>> BCM281XX for the Kconfig option.
>
> Yes, if there is some consensus that this is what we want to do.
>
> I can certainly merge that during the 3.14-rc phase for that matter.

Hi Linus,

I'll confirm the new name with Matt before sending out another patch.
Which would work better for you - a) a set of patches replacing my
previous ones, b) a set of patches on top of my previous ones, c) a
single patch on top of my previous ones? d) something else?

Thanks,
Sherman

2014-01-21 12:35:12

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On Mon, Jan 20, 2014 at 8:14 PM, Sherman Yin <[email protected]> wrote:

> I'll confirm the new name with Matt before sending out another patch. Which
> would work better for you - a) a set of patches replacing my previous ones,
> b) a set of patches on top of my previous ones, c) a single patch on top of
> my previous ones? d) something else?

(c)

The tree goes upstream now, you have to make this fix on top.

Yours,
Linus Walleij

2014-01-21 13:49:45

by Matt Porter

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: pinctrl: Add Broadcom Capri pinctrl driver

On Mon, Jan 20, 2014 at 09:16:24AM +0100, Linus Walleij wrote:
> On Sat, Jan 18, 2014 at 3:56 AM, Matt Porter <[email protected]> wrote:
>
> > I wonder if Linus would accept a rename at this point (too late for 3.14
> > presumably, but for 3.15) of s/capri/bcm281xx throughout, bcm11351 for
> > the compatible string, as we have for the machine compatible, and also
> > BCM281XX for the Kconfig option.
>
> Yes, if there is some consensus that this is what we want to do.
>
> I can certainly merge that during the 3.14-rc phase for that matter.

Ok, sounds great, thanks.

-Matt