Re: [PATCH 06/24] arm64: dts: qcom: glymur: Enable pdp0 mailbox

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 10/9/25 12:43 PM, Sibi Sankar wrote:
> 
> On 9/25/2025 3:59 PM, Konrad Dybcio wrote:
>> On 9/25/25 8:32 AM, Pankaj Patil wrote:
>>> From: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>
>>> Enable pdp0 mailbox node on Glymur SoCs.
>>>
>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>> Signed-off-by: Pankaj Patil <pankaj.patil@oss.qualcomm.com>
>>> ---
>>>   arch/arm64/boot/dts/qcom/glymur.dtsi | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> index 66a548400c720474cde8a8b82ee686be507a795f..ae013c64e096b7c90c0aa4cfc50f078a85518acb 100644
>>> --- a/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
>>> @@ -4065,6 +4065,14 @@ watchdog@17600000 {
>>>               interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>;
>>>           };
>>>   +        pdp0_mbox: mailbox@17610000 {
>>> +            compatible = "qcom,glymur-cpucp-mbox", "qcom,x1e80100-cpucp-mbox";
>>> +            reg = <0 0x17610000 0 0x8000>, <0 0x19980000 0 0x8000>;
>> 1 a line, please
> 
> Hey Konrad,
> 
> Thanks for taking time to review the series :)
> 
> Will fix it in the next re-spin.
> 
>>> +            interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
>> I see this has 3 channels, with 3 separate IRQs (but one pair of address
>> spaces) - should we extend this description?
> 
> It has a single IRQ and each bit corresponds to a channel.  The mbox theoretically
> 
> hold as many channel as the number of bits. The third channel here is used for
> 
> logging and is disabled on devices out in the wild.

Your mailing client injects two '\n's every time you press enter
Try setting mailnews.wraplength = 0 in your presumably thunderbird config


Is the logging channel useful for us, on internal devices? We can still
describe it if so


> 
>>
>>> +            #mbox-cells = <1>;
>>> +            qcom,rx-chans = <0x7>;
>> This further seems to confirm what I found (BIT(0) | BIT(1) | BIT(2) == 0x7)
>> however this property doesn't exist upstream..
> 
> Ack, this seems to have picked up erroneously and isn't needed upstream and
> 
> can be dropped safely. This was needed downstream because they share the
> 
> same rx register space across multiple instances. This wouldn't be possible
> 
> upstream and we would be exposing all mailboxes that uses the rx space in
> 
> the same instance and extend mbox cells to 2 to support this in case when
> 
> it is needed in the future.

This won't fly, as you're essentially saying you're introducing knowingly
incomplete bindings, which are supposed to stay immutable..

Konrad