All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
To: Krzysztof Kozlowski <krzk@kernel.org>, Conor Dooley <conor@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Detlev Casanova <detlev.casanova@collabora.com>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Hans Verkuil <hverkuil@kernel.org>,
	kernel@collabora.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}
Date: Wed, 4 Mar 2026 23:26:30 +0200	[thread overview]
Message-ID: <cce68679-8dcf-4c8d-8a66-df426c037f6c@collabora.com> (raw)
In-Reply-To: <a70731e8-375a-4ba9-b142-600b92ae1087@collabora.com>

On 3/3/26 2:26 AM, Cristian Ciocaltea wrote:
> Hi Krzysztof,
> 
> On 2/28/26 11:58 AM, Krzysztof Kozlowski wrote:
>> On 28/02/2026 10:54, Krzysztof Kozlowski wrote:
>>> On 27/02/2026 18:42, Cristian Ciocaltea wrote:
>>>> On 2/27/26 7:13 PM, Conor Dooley wrote:
>>>>> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>>>>>> Hi Krzysztof, Conor,
>>>>>>
>>>>>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>>>>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>>>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>>>>>> register blocks have been provided for the video decoder unit.
>>>>>>>>
>>>>>>>> However, the binding does not properly describe the new hardware layout,
>>>>>>>
>>>>>>> As you shown me last time with excerpt of address spaces from
>>>>>>> datasheet/manual, the binding correctly describes the hardware and above
>>>>>>> sentence is not true.
>>>>>>>
>>>>>>>> as it breaks the convention expecting the unit address to indicate the
>>>>>>>> start of the first register range, i.e. 'function' block is listed
>>>>>>>
>>>>>>> Imprecise wording. "start of the main or primary register range"
>>>>>>>
>>>>>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>>>>>> unit address will be 0x20000000).
>>>>>>>
>>>>>>>> before 'link' instead of the opposite.
>>>>>>>>
>>>>>>>> Since the binding changes have been already released and a fix would
>>>>>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>>>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>>>>>> which follows the address-based ordering according to the TRM.
>>>>>>>>
>>>>>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>>>>>> anymore, while the information they offer is not very relevant anyway.
>>>>>>>
>>>>>>> This is fine for me.
>>>>>>
>>>>>> Thanks for the additional feedback!
>>>>>>
>>>>>> If I'm not mistaken (please correct me), the only remaining (hard)
>>>>>> blocker for the series would be to improve this commit message.
>>>>>
>>>>> No, you also need to fix the problem I pointed out about reg-names being
>>>>> optional on the devices you're relying on reg-names for. 
>>>>
>>>> My only concern is that by marking reg-names as required we would break the ABI,
>>>
>>> You are ALREADY BREAKING the ABI. Really, for absolutely non-important
>>> cosmetic change in unit address, where I asked you repeatedly to fix the
>>> unit address, you change the ABI affecting kernel and DTS users.
> 
> I thought we've already reached consensus to allow extending the binding and
> keep both lists, precisely to avoid breaking the ABI.  At least this was my
> understanding according to your reply [1]:
> 
>   You can have also oneOf with older list "deprecated: true", if want to
>   keep any users unaffected.
> 
> And this patch was meant to do exactly that.  Did I miss something?
> 
>>> This is barely acceptable, but I am just annoyed already explain it to
>>> you multiple times.
> 
> There is no need to explain it again, we've got your point.  We've also brought our
> arguments and I had the impression that we eventually agreed to keep the unit
> address unchanged, based on your comments [2]:
> 
>   Yes, with drop of the oneOf this would be fine.
>   I meant, the "one item option" in oneOf.
> 
> Is this not applicable anymore?
> 
>>> But now you claim, you can break ABI for cosmetic unimportant change,
> 
> No, breaking ABI wasn't our intention here.  If we put the issue with reg-names
> being optional aside for a moment (as that one will be handled separately), is
> there still a problem with the current revision?
> 
>>> but actually doing something meaningful is a no-go?
> 
> Making reg-names mandatory has been already clarified with Conor and agreed [3]
> to be handled in a dedicated patch.  And that one will indeed break the ABI, but
> it's unavoidable, unfortunately. 
> 
>>> At least use correct arguments if you want to discuss.
> 
> Sorry, I'm not sure what do you mean.  I really believed that we managed to
> address all the open topics by now.

I've just submitted v5.  For some reason the link to the cover letter [1]
doesn't seem to work, I'm getting:

  Message-ID <20260304-vdec-reg-order-rk3576-v5-0-7006fad42c3a@collabora.com>
  not found

But all the others are just fine, e.g. [2] is the for the 1st patch.  I've never
encountered something similar before.

Regards,
Cristian

[1] https://lore.kernel.org/all/20260304-vdec-reg-order-rk3576-v5-0-7006fad42c3a@collabora.com/
[2] https://lore.kernel.org/all/20260304-vdec-reg-order-rk3576-v5-1-7006fad42c3a@collabora.com/





WARNING: multiple messages have this Message-ID (diff)
From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
To: Krzysztof Kozlowski <krzk@kernel.org>, Conor Dooley <conor@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Detlev Casanova <detlev.casanova@collabora.com>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Hans Verkuil <hverkuil@kernel.org>,
	kernel@collabora.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88}
Date: Wed, 4 Mar 2026 23:26:30 +0200	[thread overview]
Message-ID: <cce68679-8dcf-4c8d-8a66-df426c037f6c@collabora.com> (raw)
In-Reply-To: <a70731e8-375a-4ba9-b142-600b92ae1087@collabora.com>

On 3/3/26 2:26 AM, Cristian Ciocaltea wrote:
> Hi Krzysztof,
> 
> On 2/28/26 11:58 AM, Krzysztof Kozlowski wrote:
>> On 28/02/2026 10:54, Krzysztof Kozlowski wrote:
>>> On 27/02/2026 18:42, Cristian Ciocaltea wrote:
>>>> On 2/27/26 7:13 PM, Conor Dooley wrote:
>>>>> On Fri, Feb 27, 2026 at 01:37:17PM +0200, Cristian Ciocaltea wrote:
>>>>>> Hi Krzysztof, Conor,
>>>>>>
>>>>>> On 2/27/26 9:46 AM, Krzysztof Kozlowski wrote:
>>>>>>> On Thu, Feb 26, 2026 at 12:46:53PM +0200, Cristian Ciocaltea wrote:
>>>>>>>> With the introduction of the RK3588 SoC, and RK3576 afterwards, two more
>>>>>>>> register blocks have been provided for the video decoder unit.
>>>>>>>>
>>>>>>>> However, the binding does not properly describe the new hardware layout,
>>>>>>>
>>>>>>> As you shown me last time with excerpt of address spaces from
>>>>>>> datasheet/manual, the binding correctly describes the hardware and above
>>>>>>> sentence is not true.
>>>>>>>
>>>>>>>> as it breaks the convention expecting the unit address to indicate the
>>>>>>>> start of the first register range, i.e. 'function' block is listed
>>>>>>>
>>>>>>> Imprecise wording. "start of the main or primary register range"
>>>>>>>
>>>>>>> (if you have 0x1000 with one reg and 0x20000000 with everything, the
>>>>>>> unit address will be 0x20000000).
>>>>>>>
>>>>>>>> before 'link' instead of the opposite.
>>>>>>>>
>>>>>>>> Since the binding changes have been already released and a fix would
>>>>>>>> bring up an ABI break, mark the current 'reg-names' ordering as
>>>>>>>> deprecated and introduce an alternative 'link,function,cache' listing
>>>>>>>> which follows the address-based ordering according to the TRM.
>>>>>>>>
>>>>>>>> Additionally, drop the 'reg' description items as the order is not fixed
>>>>>>>> anymore, while the information they offer is not very relevant anyway.
>>>>>>>
>>>>>>> This is fine for me.
>>>>>>
>>>>>> Thanks for the additional feedback!
>>>>>>
>>>>>> If I'm not mistaken (please correct me), the only remaining (hard)
>>>>>> blocker for the series would be to improve this commit message.
>>>>>
>>>>> No, you also need to fix the problem I pointed out about reg-names being
>>>>> optional on the devices you're relying on reg-names for. 
>>>>
>>>> My only concern is that by marking reg-names as required we would break the ABI,
>>>
>>> You are ALREADY BREAKING the ABI. Really, for absolutely non-important
>>> cosmetic change in unit address, where I asked you repeatedly to fix the
>>> unit address, you change the ABI affecting kernel and DTS users.
> 
> I thought we've already reached consensus to allow extending the binding and
> keep both lists, precisely to avoid breaking the ABI.  At least this was my
> understanding according to your reply [1]:
> 
>   You can have also oneOf with older list "deprecated: true", if want to
>   keep any users unaffected.
> 
> And this patch was meant to do exactly that.  Did I miss something?
> 
>>> This is barely acceptable, but I am just annoyed already explain it to
>>> you multiple times.
> 
> There is no need to explain it again, we've got your point.  We've also brought our
> arguments and I had the impression that we eventually agreed to keep the unit
> address unchanged, based on your comments [2]:
> 
>   Yes, with drop of the oneOf this would be fine.
>   I meant, the "one item option" in oneOf.
> 
> Is this not applicable anymore?
> 
>>> But now you claim, you can break ABI for cosmetic unimportant change,
> 
> No, breaking ABI wasn't our intention here.  If we put the issue with reg-names
> being optional aside for a moment (as that one will be handled separately), is
> there still a problem with the current revision?
> 
>>> but actually doing something meaningful is a no-go?
> 
> Making reg-names mandatory has been already clarified with Conor and agreed [3]
> to be handled in a dedicated patch.  And that one will indeed break the ABI, but
> it's unavoidable, unfortunately. 
> 
>>> At least use correct arguments if you want to discuss.
> 
> Sorry, I'm not sure what do you mean.  I really believed that we managed to
> address all the open topics by now.

I've just submitted v5.  For some reason the link to the cover letter [1]
doesn't seem to work, I'm getting:

  Message-ID <20260304-vdec-reg-order-rk3576-v5-0-7006fad42c3a@collabora.com>
  not found

But all the others are just fine, e.g. [2] is the for the 1st patch.  I've never
encountered something similar before.

Regards,
Cristian

[1] https://lore.kernel.org/all/20260304-vdec-reg-order-rk3576-v5-0-7006fad42c3a@collabora.com/
[2] https://lore.kernel.org/all/20260304-vdec-reg-order-rk3576-v5-1-7006fad42c3a@collabora.com/




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  reply	other threads:[~2026-03-04 21:26 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-26 10:46 [PATCH v4 0/3] arm64: dts: rockchip: Fix vdec register blocks order on RK3576/RK3588 Cristian Ciocaltea
2026-02-26 10:46 ` Cristian Ciocaltea
2026-02-26 10:46 ` [PATCH v4 1/3] media: dt-bindings: rockchip,vdec: Add alternative reg-names order for RK35{76,88} Cristian Ciocaltea
2026-02-26 10:46   ` Cristian Ciocaltea
2026-02-26 18:43   ` Conor Dooley
2026-02-26 18:43     ` Conor Dooley
2026-02-26 19:45     ` Nicolas Dufresne
2026-02-26 19:45       ` Nicolas Dufresne
2026-02-26 20:59       ` Conor Dooley
2026-02-26 20:59         ` Conor Dooley
2026-02-26 21:56         ` Nicolas Dufresne
2026-02-26 21:56           ` Nicolas Dufresne
2026-02-26 22:15           ` Conor Dooley
2026-02-26 22:15             ` Conor Dooley
2026-02-26 22:41             ` Nicolas Dufresne
2026-02-26 22:41               ` Nicolas Dufresne
2026-02-27  7:38             ` Krzysztof Kozlowski
2026-02-27  7:38               ` Krzysztof Kozlowski
2026-02-27  9:09               ` Conor Dooley
2026-02-27  9:09                 ` Conor Dooley
2026-02-27 17:18           ` Conor Dooley
2026-02-27 17:18             ` Conor Dooley
2026-02-27 17:49             ` Cristian Ciocaltea
2026-02-27 17:49               ` Cristian Ciocaltea
2026-02-27 18:10               ` Conor Dooley
2026-02-27 18:10                 ` Conor Dooley
2026-02-27 19:35                 ` Cristian Ciocaltea
2026-02-27 19:35                   ` Cristian Ciocaltea
2026-02-27 19:39                   ` Conor Dooley
2026-02-27 19:39                     ` Conor Dooley
2026-02-27  7:39     ` Krzysztof Kozlowski
2026-02-27  7:39       ` Krzysztof Kozlowski
2026-02-27  7:46   ` Krzysztof Kozlowski
2026-02-27  7:46     ` Krzysztof Kozlowski
2026-02-27 11:37     ` Cristian Ciocaltea
2026-02-27 11:37       ` Cristian Ciocaltea
2026-02-27 13:03       ` Krzysztof Kozlowski
2026-02-27 13:03         ` Krzysztof Kozlowski
2026-02-28  1:11         ` Nicolas Dufresne
2026-02-28  1:11           ` Nicolas Dufresne
2026-02-27 17:13       ` Conor Dooley
2026-02-27 17:13         ` Conor Dooley
2026-02-27 17:42         ` Cristian Ciocaltea
2026-02-27 17:42           ` Cristian Ciocaltea
2026-02-28  9:54           ` Krzysztof Kozlowski
2026-02-28  9:54             ` Krzysztof Kozlowski
2026-02-28  9:58             ` Krzysztof Kozlowski
2026-02-28  9:58               ` Krzysztof Kozlowski
2026-03-03  0:26               ` Cristian Ciocaltea
2026-03-03  0:26                 ` Cristian Ciocaltea
2026-03-04 21:26                 ` Cristian Ciocaltea [this message]
2026-03-04 21:26                   ` Cristian Ciocaltea
2026-02-26 10:46 ` [PATCH v4 2/3] arm64: dts: rockchip: Fix vdec register blocks order on RK3576 Cristian Ciocaltea
2026-02-26 10:46   ` Cristian Ciocaltea
2026-02-26 10:46 ` [PATCH v4 3/3] arm64: dts: rockchip: Update vdec register blocks order on RK3588 Cristian Ciocaltea
2026-02-26 10:46   ` Cristian Ciocaltea

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=cce68679-8dcf-4c8d-8a66-df426c037f6c@collabora.com \
    --to=cristian.ciocaltea@collabora.com \
    --cc=conor+dt@kernel.org \
    --cc=conor.dooley@microchip.com \
    --cc=conor@kernel.org \
    --cc=detlev.casanova@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=heiko@sntech.de \
    --cc=hverkuil@kernel.org \
    --cc=kernel@collabora.com \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mchehab@kernel.org \
    --cc=nicolas.dufresne@collabora.com \
    --cc=robh@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.