From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
Conor Dooley <conor@kernel.org>
Cc: Mehdi Djait <mehdi.djait@bootlin.com>,
mchehab@kernel.org, heiko@sntech.de, hverkuil-cisco@xs4all.nl,
krzysztof.kozlowski+dt@linaro.org, robh+dt@kernel.org,
conor+dt@kernel.org, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
thomas.petazzoni@bootlin.com, alexandre.belloni@bootlin.com,
maxime.chevallier@bootlin.com, michael.riesch@wolfvision.net
Subject: Re: [PATCH v10 1/3] media: dt-bindings: media: add bindings for Rockchip CIF
Date: Thu, 9 Nov 2023 18:53:12 +0100 [thread overview]
Message-ID: <e5b1f0dd-0aab-4ce5-82ba-879a4d736e7e@linaro.org> (raw)
In-Reply-To: <ZU0avuRRaITV4jws@aptenodytes>
On 09/11/2023 18:45, Paul Kocialkowski wrote:
> Hi,
>
> On Thu 09 Nov 23, 17:24, Conor Dooley wrote:
>> On Wed, Nov 08, 2023 at 05:38:56PM +0100, Mehdi Djait wrote:
>>> Add a documentation for the Rockchip Camera Interface binding.
>>>
>>> the name of the file rk3066 is the first Rockchip SoC generation that uses cif
>>> instead of the px30 which is just one of the many iterations of the unit.
>>
>> I think this is becoming ridiculous. You've now removed the compatible
>> for the rk3066 but kept it in the filename. I don't understand the
>> hangup about naming the file after the px30-vip, but naming it after
>> something that is not documented here at all makes no sense to me.
>> Either document the rk3066 properly, or remove all mention of it IMO.
>
> I think the opposite is ridiculous. We have spent some time investigating the
> history of this unit, to find out that RK3066 is the first occurence where
> it exists. Since we want the binding to cover all generations of the same unit
> and give it a name that reflects this, rk3066 is the natural choice that comes
> to mind. As far as I understand, this is the normal thing to do to name
> bindings: name after the earliest known occurence of the unit.
>
> What is the rationale behind naming the file after a generation of the unit
> that happens to be the one introducing the binding? This is neither the first
> nor the last one to include this unit. The binding will be updated later to
> cover other generations. Do we want to rename the file each time an a generation
> earlier than px30 is introduced? That sounds quite ridiculous too.
>
> We've done the research work to give it the most relevant name here.
> I'd expect some strong arguments not to use it. Can you ellaborate?
If you do not have rk3066 documented here, it might be added to entirely
different file (for whatever reasons, including that binding would be
quite different than px30). Thus you would have rk3066 in
rockchip,rk3066-cif-added-later.yaml and px30 in rockchip,rk3066-cif.yaml
Just use the filename matching the compatible. That's what we always
ask. In every review.
Best regards,
Krzysztof
next prev parent reply other threads:[~2023-11-09 17:53 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-08 16:38 [PATCH v10 0/3] media: rockchip: Add a driver for Rockchip's camera interface Mehdi Djait
2023-11-08 16:38 ` [PATCH v10 1/3] media: dt-bindings: media: add bindings for Rockchip CIF Mehdi Djait
2023-11-09 17:24 ` Conor Dooley
2023-11-09 17:45 ` Paul Kocialkowski
2023-11-09 17:53 ` Krzysztof Kozlowski [this message]
2023-11-09 18:07 ` Paul Kocialkowski
2023-11-10 18:23 ` Conor Dooley
2023-11-13 16:54 ` Paul Kocialkowski
2023-11-14 17:51 ` Conor Dooley
2023-11-15 15:07 ` Paul Kocialkowski
2023-11-15 20:16 ` Conor Dooley
2023-11-08 16:38 ` [PATCH v10 2/3] media: rockchip: Add a driver for Rockchip's camera interface Mehdi Djait
2023-11-10 12:50 ` Michael Riesch
2023-11-13 11:06 ` Mehdi Djait
2023-11-13 15:41 ` Michael Riesch
2023-11-10 14:33 ` Michael Riesch
2023-11-13 15:28 ` Mehdi Djait
2023-11-13 16:05 ` Michael Riesch
2023-11-15 9:16 ` Mehdi Djait
2023-11-08 16:38 ` [PATCH v10 3/3] arm64: dts: rockchip: Add the " Mehdi Djait
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e5b1f0dd-0aab-4ce5-82ba-879a4d736e7e@linaro.org \
--to=krzysztof.kozlowski@linaro.org \
--cc=alexandre.belloni@bootlin.com \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=heiko@sntech.de \
--cc=hverkuil-cisco@xs4all.nl \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=maxime.chevallier@bootlin.com \
--cc=mchehab@kernel.org \
--cc=mehdi.djait@bootlin.com \
--cc=michael.riesch@wolfvision.net \
--cc=paul.kocialkowski@bootlin.com \
--cc=robh+dt@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®