Re: [PATCH] arm64: dts: allwinner: a64-oceanic-5205-5inmfd: Enable CAN

[Michael Nazzareno Trimarchi]

Hi Maxime

On Tue, May 21, 2019 at 10:10 AM Maxime Ripard
<maxime.ripard@xxxxxxxxxxx> wrote:
>
> On Tue, May 21, 2019 at 08:47:02AM +0200, Michael Nazzareno Trimarchi wrote:
> > > > +     };
> > > > +
> > > >  };
> > > >
> > > >  &ehci0 {
> > > > @@ -77,6 +95,31 @@
> > > >       status = "okay";
> > > >  };
> > > >
> > > > +&pio {
> > > > +     can_pins: can-pins {
> > > > +             pins = "PD6",                   /* RX_BUF1_CAN0 */
> > > > +                    "PD7";                   /* RX_BUF0_CAN0 */
> > > > +             function = "gpio_in";
> > > > +     };
> > > > +};
> > >
> > > That isn't needed. What are they used for, you're not tying them to
> > > anything?
> >
> > Mux of their function is correct. They are connected in the schematics
> > but not used right now.
>
> Then describe the whole thing or don't?
>

Ok

> And that's kind of missing my point. If that pin group isn't related
> to any device, the pin muxing will not be changed. So that group, in
> itself, has strictly no effect.
>
> Moreover, you don't need a pin group in the first place to mux pins in
> GPIOs, the GPIO API will make sure that is the case when you request
> it.

This is correct on sunxi. Is this valid for sunxi or in general in all the SoC?
Anyway make sense to have pins configured and place in the right
state, just suppose if the
booting stage is wrong or anything that make those pins in the wrong
configuration

>
> > I can garantee that kernel wlll always configurred in the right way
> > and if I want I can export in userspace
> > for debug purpose

Correct if you start to use it but if you want them right configured
the right place
is in the default state e/o initstate if this can be a problem of the hardware

Default state: the state the pinctrl handle shall be put
 *      into as default, usually this means the pins are up and ready to
 *      be used by the device driver. This state is commonly used by
 *      hogs to configure muxing and pins at boot, and also as a state
 *      to go into when returning from sleep and idle in
 *      .pm_runtime_resume() or ordinary .resume() for example.

Now the pins are connected to the canbus as should be and they are
configured and usually
put in the right state.

+               compatible = "microchip,mcp2515";
+               reg = <0>;
+               spi-max-frequency = <10000000>;
+               pinctrl-names = "default";
+               pinctrl-0 = <&can_pins>;

>
> Yes, because the API does it, not your change
>

Do you prefer to drop the pinmux? or update the commit message

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |