Re: [PATCH v1 01/10] dt-bindings: media: Add Renesas CEU bindings

From: Geert Uytterhoeven
Date: Wed Nov 15 2017 - 13:39:33 EST


Hi Jacopo,

On Wed, Nov 15, 2017 at 7:15 PM, jacopo mondi <jacopo@xxxxxxxxxx> wrote:
> On Wed, Nov 15, 2017 at 02:07:31PM +0100, Geert Uytterhoeven wrote:
>> On Wed, Nov 15, 2017 at 11:55 AM, Jacopo Mondi
>> <jacopo+renesas@xxxxxxxxxx> wrote:
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
>> > @@ -0,0 +1,87 @@
>> > +Renesas Capture Engine Unit (CEU)
>> > +----------------------------------------------
>> > +
>> > +The Capture Engine Unit is the image capture interface found on Renesas
>> > +RZ chip series and on SH Mobile ones.
>> > +
>> > +The interface supports a single parallel input with up 8/16bits data bus width.
>>
>> ... with data bus widths up to 8/16 bits?
>>
>> > +
>> > +Required properties:
>> > +- compatible
>> > + Must be "renesas,renesas-ceu".
>>
>> The double "renesas" part looks odd to me. What about "renesas,ceu"?
>
> I'm totally open for better "compatible" strings here, so yeah, let's
> got for the shorter one you proposed...
>
>> Shouldn't you add SoC-specific compatible values like "renesas,r7s72100-ceu",
>> too?
>
> Well, I actually have no SoC-specific data in the driver, so I don't
> need SoC specific "compatible" values. But if it's a good practice
> to have them anyway, I will add those in next spin..

You don't necessarily need them in the driver, but in the bindings and DTS,
just in case a difference is discovered later.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds