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

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

Hello Diederik,

On Wednesday, 4 June 2025 10:24:33 Central European Summer Time Diederik de Haas wrote:
> Hi Nicolas,
> 
> 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.

Kind regards,
Nicolas Frattaroli

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