Hi, Maxime, Thanks for your comments. On Mon, 15 Mar 2021 16:29:04 +0100 Maxime Ripard <maxime@xxxxxxxxxx> wrote: > Hi, > > You seem to have some issues with your mailer that mangles the mail, > you should try using git-send-email instead. Will do. > There's also a bunch of checkpatch --strict issues you want to get rid > of. Thanks for pointing this out. Besides the style warnings, which I've fixed, there are also warnings pertaining to documentation. Should I add the compatible strings to Documentation/devicetree/bindings/, and/or myself to the MAINTAINERS file? If so, would you suggest that I do so in a separate patch, or as part of another patch that adds the relevant files? > On Thu, Mar 11, 2021 at 03:30:56PM +0300, Ivan Uvarov wrote: > > Add support for the Forlinx FETA40i-C SoM and OKA40i-C devboard[1] > > based on it. The DT is split into a .dtsi, which (hopefully) > > corresponds to the functions of the SoM itself, and a .dts for the > > devboard. > > > > [1]:https://linux-sunxi.org/Forlinx_OKA40i-C > > Instead of a URL that might be dead at some point, it would be better > to have a proper commit log explaining why you did your patch that way I'm not sure I understand. By "that way" do you mean that I should explain why I've split up the devicetree, or something else? The linked page is just a collection of information about the device; should I describe the SoM and/or devboard in my commit log instead? > > ... > > @@ -601,6 +603,14 @@ mmc2_pins: mmc2-pins { > > bias-pull-up; > > }; > > > > + mmc3_pins: mmc3-pins { > > + pins = "PI4", "PI5", "PI6", > > + "PI7", "PI8", "PI9"; > > + function = "mmc3"; > > + drive-strength = <30>; > > + bias-pull-up; > > + }; > > + > > This should be in a separate patch > > > /omit-if-no-ref/ > > spi0_pc_pins: spi0-pc-pins { > > pins = "PC0", "PC1", "PC2"; > > @@ -636,6 +646,16 @@ uart0_pb_pins: uart0-pb-pins { > > function = "uart0"; > > }; > > > > + uart2_pi_pins: uart2-pi-pins { > > + pins = "PI18", "PI19"; > > + function = "uart2"; > > + }; > > + > > + uart2_rts_cts_pi_pins: > > uart2-rts-cts-pi-pins{ > > + pins = "PI16", "PI17"; > > + function = "uart2"; > > + }; > > + > > Ditto, and it should have /omit-if-no-ref/ Should the MMC pins be in the same patch as the UART pins? Also, would it be a good idea to add /omit-if-no-ref/ to the uart3 pins as well while I'm here? > > uart3_pg_pins: uart3-pg-pins { > > pins = "PG6", "PG7"; > > function = "uart3"; > > @@ -645,6 +665,21 @@ uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { > > pins = "PG8", "PG9"; > > function = "uart3"; > > }; > > + > > + uart4_pg_pins: uart4-pg-pins { > > + pins = "PG10", "PG11"; > > + function = "uart4"; > > + }; > > + > > + uart5_ph_pins: uart5-ph-pins { > > + pins = "PH6", "PH7"; > > + function = "uart5"; > > + }; > > + > > + uart7_pi_pins: uart7-pi-pins { > > + pins = "PI20", "PI21"; > > + function = "uart7"; > > + }; > > Ditto > > maxime Since this won't be a monolithic patch anymore, would it make more sense to make this a three+-patch series: 1. add board+SOM DT without mmc3 or extra uarts; 2+. add pins to r40.dtsi; 3. add mmc3&uarts to the devboard DT, or to make only two(+) patches: 1+. add pins to r40; 2. add board+SOM DT? -- Regards, Ivan Uvarov
