Re: [PATCH v3 3/3] ARM: dts: Add Caninos Loucos Labrador

[Matheus Castello]

Hi Andreas,

thanks for the review.

Em 4/26/20 8:58 AM, Andreas FÃrber escreveu:
> Hi Matheus,
>
> Am 23.04.20 um 17:20 schrieb Matheus Castello:
> > Em 4/5/20 3:51 AM, Manivannan Sadhasivam escreveu:
> > > On Fri, Mar 20, 2020 at 12:51:04AM -0300, Matheus Castello wrote:
> > > > Add Device Trees for Caninos Loucos Labrador CoM and base board.
> > > > Based on the work of Andreas FÃrber on Lemaker Guitar device tree.
> > > >
> > > > Signed-off-by: Matheus Castello <matheus@xxxxxxxxxxxxxxx>
> > >
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > >
> > > Will queue the series for v5.8 once Andreas is happy with it.
> >
> > do you have any more concerns about these patches? Let me know.
>
> I've reviewed the preceding two patches. This one here looks okay,
>
> Reviewed-by: Andreas FÃrber <afaerber@xxxxxxx>
>
> but see 2/3 - model here does contain M and your .dtsi is named -v2 - 
> what's the difference to v1? If it's big enough to warrant a separate 
> .dtsi, you should consider whether a versioned compatible string may be 
> needed, too (likely in addition to, not instead of a generic one). No v1 
> info on the website.

The difference between v1 and v2 is the model and vendor of the DRAM and 
storage eMMC. But that had no impact on the software.

Checking examples on "/Documentation/devicetree/bindings/arm/" I saw 
that some add enum descriptions for both the base board and system on 
module, keeping in const only the vendor prefix of the SoC. And in the 
device tree both have the model property description. I liked this 
example, I think I will follow it in v4.

About "-bb-" in the file name and description: on PCB is written 
"Labrador Base-M v1.0a", so with that I think it is better to leave only 
base on the name.

I will send the v4, thank you very much for the review.

BR,
Matheus Castello

> Thanks,
> Andreas