[v10,1/3] media: dt-bindings: media: add bindings for Rockchip CIF

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.

Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
---
 .../bindings/media/rockchip,rk3066-cif.yaml   | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip,rk3066-cif.yaml

Hi,

On Thu 09 Nov 23, 18:53, Krzysztof Kozlowski wrote:
> 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

As far as I could see we generally manage to include support for different
hardware setups in the same binding document using conditionals on the
compatible, so this feels a bit far-fetched.

Of course you're the maintainer and have significantly more experience here
so there might be a lot that I'm not seeing, but I'm not very convinced by this
reasoning to be honest.

> Just use the filename matching the compatible. That's what we always
> ask. In every review.

Yeah and we very often end up with naming that is less than optimal (to stay
polite). I'm generally quite appalled by the overall lack of interest that
naming gets, as if it was something secondary. Naming is one of the most
important and difficult things in our field of work and it needs to be
considered with care.

This is not just a problem with device-tree, it's a kernel-wide issue that
nobody seems to be interested in addressing. I'm quite unhappy to see that when
time is spent trying to improve the situation on one particular instance, we are
shown the door because it doesn't match what is generally done (and often done
wrong).

This is definitely a rant. I really want to express this issue loud and clear
and encourage everyone to consider it for what it is.

Cheers,

Paul

On Thu, Nov 09, 2023 at 07:07:27PM +0100, Paul Kocialkowski wrote:
> On Thu 09 Nov 23, 18:53, Krzysztof Kozlowski wrote:
> > On 09/11/2023 18:45, Paul Kocialkowski wrote:
> > > 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
> 
> As far as I could see we generally manage to include support for different
> hardware setups in the same binding document using conditionals on the
> compatible, so this feels a bit far-fetched.
> 
> Of course you're the maintainer and have significantly more experience here
> so there might be a lot that I'm not seeing, but I'm not very convinced by this
> reasoning to be honest.
> 
> > Just use the filename matching the compatible. That's what we always
> > ask. In every review.
> 
> Yeah and we very often end up with naming that is less than optimal (to stay
> polite). I'm generally quite appalled by the overall lack of interest that
> naming gets, as if it was something secondary. Naming is one of the most
> important and difficult things in our field of work and it needs to be
> considered with care.
> 
> This is not just a problem with device-tree, it's a kernel-wide issue that
> nobody seems to be interested in addressing. I'm quite unhappy to see that when
> time is spent trying to improve the situation on one particular instance, we are
> shown the door because it doesn't match what is generally done (and often done
> wrong).
> 
> This is definitely a rant. I really want to express this issue loud and clear
> and encourage everyone to consider it for what it is.

Look chief, I do understand your frustration here, with the seemingly
arbitrary naming etc. I'm apologise if using the word "ridiculous" earlier
pissed you off. I'm sure you can similarly understand why we don't want
to accept either having a compatible for the rk3066-cif in the file,
when you are not yet sure of the correct constraints, or given your
interest in naming, why calling it after something that it does not even
document is misleading.
Ultimately, I don't care what the file ends up being called when there
are multiple devices documented in it. I'd ack a patch renaming to the
œriginal incarnation of the IP when the documentation for that IP is
added without a second thought.

On Mon, Nov 13, 2023 at 05:54:08PM +0100, Paul Kocialkowski wrote:
 
> > Ultimately, I don't care what the file ends up being called when there
> > are multiple devices documented in it. I'd ack a patch renaming to the
> > œriginal incarnation of the IP when the documentation for that IP is
> > added without a second thought.
> 
> That would be agreeable to me if my proposal still ends up feeling unreasonable
> to you. But I might very well take you at your word since I ended up purchasing
> a RK3066 board in a moment of weakness last week.

The ideal outcome I suppose would be documenting both variants. If
you've gone ahead and bought one, give that a go.

On Wed, Nov 15, 2023 at 04:07:07PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Tue 14 Nov 23, 17:51, Conor Dooley wrote:
> > On Mon, Nov 13, 2023 at 05:54:08PM +0100, Paul Kocialkowski wrote:
> >  
> > > > Ultimately, I don't care what the file ends up being called when there
> > > > are multiple devices documented in it. I'd ack a patch renaming to the
> > > > œriginal incarnation of the IP when the documentation for that IP is
> > > > added without a second thought.
> > > 
> > > That would be agreeable to me if my proposal still ends up feeling unreasonable
> > > to you. But I might very well take you at your word since I ended up purchasing
> > > a RK3066 board in a moment of weakness last week.
> > 
> > The ideal outcome I suppose would be documenting both variants. If
> > you've gone ahead and bought one, give that a go.
> 
> Yeah I'll try to do that eventually, but we really want to have this series
> merged as soon as possible. So it wouldn't be reasonable for us to wait for
> RK3066 support.
> 
> What's your final decision for now: is it okay to keep the file named
> rockchip,rk3066-cif.yaml (without this compatible in the file) or do you still
> want it called rockchip,px30-vip.yaml?

I'd still rather it was called after the only compatible in the file and
subsequently renamed, sorry.