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". > That said: ASUS, Cisco, Compex, Netgear, Zyxel... opted to either > 1. export the individual GPIOs with sysfs > 2. gpio-leds > For any software-driven LED control it makes sense to configure the pins as gpio and just rely on the gpio-leds driver. But as you have both hardware driven control and hardware blink support it sounds like there's good reason to have a specific LED driver as well. > > [..] > > > + qca_mux_wifi01, > > > > Please make these "wifi0" and include all "wifi0XY", rather than having > > a group per pin. > > > > > + qca_mux_wifi11, > > > > "wifi1" > > Ok. Can I leave wifi1_cal, _wci, uartX... the way they are? > Yes, that sounds reasonable > > > + qca_mux_atest_char3, > > > + qca_mux_pmu0, > > > + qca_mux_boot8, > > > + qca_mux_tm1, > > > + qca_mux_atest_char2, > > > + qca_mux_pmu1, > > > + qca_mux_boot9, > > > + qca_mux_tm2, > > > + qca_mux_atest_char1, > > > + qca_mux_tm_ack, > > > + qca_mux_wifi03, > > > + qca_mux_wifi13, > > > + 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. > By the way, can you ask if the QCA8074 follows the IPQ40XX SoC's > GPIO Pull-up config? > > I'm asking because Varadarajan was also involved in the > pinctrl-ipq4019 back in 2015: <https://patchwork.kernel.org/patch/7662241/> > > Back then, this wasn't mentioned anywhere. In fact, the special pull-up > configuration was only discovered due to an issue with the NAND on the > Cisco Meraki MR33. So I think it is better ask them now, when the devs > are actually present/responding. > I do unfortunately not have access to the specification or either platform, I would appreciate if you could post a reply on Varadarajan's IPQ8074 patch asking him to verify the layout of the config register. [..] > > > + qca_mux_boot18, > > > > Do you know what the "boot" function is and what 2, 4, 5, 7, 8, 9 11, > > 14, 18, 19 and 20 means? > Sadly no. That said, neither the u-boot nor the linux kernel > sources set any pin to bootX. I think I'll remove it for now. > Better leave them out for now then. > > [..] > > > + qca_mux_sdio0, > > > > There are 8 of these, so that's more likely the 8 data pins in a single > > function. Please squash them into "sdio_data". > Yes, this is very likely. We know that this is the case for the qpic_padX. > Sounds good. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html