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

["Diederik de Haas" <didi.debian@cknow.org>]

[-- 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 --]