Re: [PATCH 2/2] PM / devfreq: rockchip-dfi: add support for LPDDR5

Diederik de Haas
2025-06-05T19:50:02Z

[-- Attachment #1: Type: text/plain, Size: 2522 bytes --]

Hi,

On Thu Jun 5, 2025 at 5:14 PM CEST, Nicolas Frattaroli wrote:
> On Wednesday, 4 June 2025 10:24:33 Central European Summer Time Diederik de Haas wrote:
>> On Fri May 30, 2025 at 3:38 PM CEST, Nicolas Frattaroli wrote:
>> > [...]
>> > + case ROCKCHIP_DDRTYPE_LPDDR5:
>> > + ddrmon_ver = readl_relaxed(dfi->regs);
>> > + if (ddrmon_ver < 0x40) {
>> > + *ctrl = DDRMON_CTRL_LPDDR5 | dfi->lp5_bank_mode;
>> > + *mask |= DDRMON_CTRL_LP5_BANK_MODE_MASK;
>> > + break;
>> > + }
>> > +
>> > + /*
>> > + * As it is unknown whether the unpleasant special case
>> > + * behaviour used by the vendor kernel is needed for any
>> > + * shipping hardware, ask users to report if they have
>> > + * some of that hardware.
>> > + */
>> > + dev_err(&dfi->edev->dev,
>> > + "unsupported DDRMON version 0x%04X, please let linux-rockchip know!\n",
>> > + ddrmon_ver);
>> > + return -EOPNOTSUPP;
>>
>> I'm guessing you mean the linux-rockchip mailing list here? If so, I
>> think it's better to make that explicit as 'Joe User' who may run into
>> this issue may not be aware of that mailing list. The 'linux' and
>> 'rockchip' name combo is used in quite a few places.
>
> I agree it's ambiguous, the message as it is right now is the way it is
> because we're not allowed to linebreak user-facing messages for grep-
> ability and I also don't want to exceed 100 lines of width (though this
> is the one case where we're allowed to).
>
> I suppose I should just replace it with the e-mail address of the list.
> That should be clear enough and this error message also won't end up in
> random boot logs strewn across the internet if it really is just some
> engineering sample hardware or similar that's affected.

I understand the issue. Maybe this discussion itself will help make it
appear high enough in the search results to fix the ambiguity ;-)
I'll leave it up to you if or how to change anything (or not).

Cheers,
Diederik

>> > + default:
>> > + dev_err(&dfi->edev->dev, "unsupported memory type 0x%X\n",
>> > + dfi->ddr_type);
>> > + return -EOPNOTSUPP;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
>> > {
>> > void __iomem *dfi_regs = dfi->regs;
>> > int i, ret = 0;
>> > + u32 ctrl;
>> > + u32 ctrl_mask;
>> >
>> > mutex_lock(&dfi->mutex);
>> >
>> > @@ -136,8 +186,11 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
>> > <snip>
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]