Re: [PATCH 2/2] pinctrl: qcom: ipq4019: add remaining pin definitions

[Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>]

On Friday, May 19, 2017 10:08:24 PM CEST Bjorn Andersson wrote:
> On Thu 18 May 11:38 PDT 2017, Christian Lamparter wrote:
> > On Wednesday, May 17, 2017 1:07:29 PM CEST Bjorn Andersson wrote:
> > > On Wed 10 May 04:27 PDT 2017, Christian Lamparter wrote:
> > > > diff --git a/drivers/pinctrl/qcom/pinctrl-ipq4019.c b/drivers/pinctrl/qcom/pinctrl-ipq4019.c
> > > > index 743d1f458205..7219d1e33c71 100644
> > > > --- a/drivers/pinctrl/qcom/pinctrl-ipq4019.c
> > > > +++ b/drivers/pinctrl/qcom/pinctrl-ipq4019.c
> > > > +	qca_mux_rmii0_refclk,
> > > > +	qca_mux_wifi0_rfsilient0,
> > > > +	qca_mux_wifi1_rfsilient0,
> > > > +	qca_mux_smart2,
> > > > +	qca_mux_led4,
> > > 
> > > What drives ledX? Is it 11 different LED controllers or is it a single
> > > LED controller with 11 outputs.
> > 
> > The latter. The IPQ40xx have one LED controller @ 0x1937000. 
> > According to the driver (leds-ipq40xx.c in the SDK), it 
> > does control up to 11 LEDs. A LED can either be muxed to
> > one of the hardware sources (wifi, lan or wan-ports activity/linkspeed),
> > or it can be operated by one of four software-programmable "blink"
> > sources (each with a variable blink rate and duty cycle). 
> > The driver labels each LED as "ipq40xx::led%d".
> > 
> 
> As they all stem from the same hardware block I suggest we name the
> function "led".

This is going to be a problem for Pin 36:

PINGROUP(36, rmii0, *led2*, *led0*, NA, ...)

I checked the other pins too, but this seems to be the only time two 
LED-lines share the same pin. What's the recommended/prefered option
in this case? Something like led_alt, or should I keep the led0-led11?

There are a few more collisions with other functions as well:
smartX, i2s_*, wifi1_uart*:

PINGROUP(58, qpic, led2, blsp_i2c0, *smart3*, *smart1*,
         i2s_rx_mclk, NA, wcss0_dbg, tm4, wifi0, wifi1, NA, NA, NA),

PINGROUP(60, qpic, blsp_uart0, smart1, smart3, led0,
         *i2s_tx_bclk*, *i2s_rx_bclk*, atest_char, NA, wcss0_dbg,
         qdss_traceclk_a, NA, tm6, NA),

PINGROUP(61, qpic, blsp_uart0, *smart1*, *smart3*, led1, *i2s_tx_fsync*,
         *i2s_rx_fsync*, NA, NA, wcss0_dbg, qdss_cti_trig_out_a0,
         NA, tm7, NA),

PINGROUP(63, qpic, wifi0_uart1, *wifi1_uart1*, *wifi1_uart*, *i2s_td1*,
         *i2s_rxd*, *i2s_spdif_out*, *i2s_spdif_in*, NA, wcss0_dbg,
         wcss1_dbg, NA, tm, NA),
...
> > > > +	qca_mux_qpic_pad4,
> > > 
> > > Please keep an eye on the ipq8074 patch from Varadarajan and make this
> > > follow the same scheme.
> > Ok, I'll wait for how qca8074 plays out then.

<https://www.spinics.net/lists/arm-kernel/msg582958.html>:
| On Sat, May 20, 2017 at 7:54 AM, Bjorn Andersson wrote:
| [...]
| > If you consider that you are defining the available functions for this
| > pinmuxer and then define the sets of pins exposing these available
| > functions it does make sense to just name it "qpic".
| >
| > I think that naming them _common, _lcd and _nand is just adding
| > confusion when it comes to writing the dts files.
| >
| > @Linus, do you have a different preference here?
|
|No I pretty much trust the driver maintainer to know this best.
|
|Yours,
|Linus Walleij

Thanks for the answer, I'll go with "qpic" then.
I'll also combine the various sdio_* to sdio and look at the other candidates
(rgmii*, rmii0/1*, pmuX, pcie_clk*, jtag*, tmX, audio_pwmX). Unless someone
comes up with a good reason not to.

Regards,
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html