Subject: Re: [PATCH v4 04/21] PM / devfreq: rockchip-dfi: Add SoC specific init function

From: Jonathan Cameron <[email-masked]>
Date: Wed, 17 May 2023 11:19:02 +0100

> >   
> > > +
> > > +	return 0;
> > > +};
> > > +
> > > +struct rockchip_dfi_devtype_data {
> > > +	int (*init)(struct rockchip_dfi *dfi);
> > > +};
> > > +
> > > +static struct rockchip_dfi_devtype_data rk3399_devtype_data = {
> > > +	.init = rk3399_dfi_init,  
> > 
> > I guess the 'why' will become apparent later in the set, but at this
> > stage a callback to get the ddr type would have made more sense and
> > kept the data local to where it was needed.  
> 
> struct rockchip_dfi_devtype_data will still have a single member by the
> end of this series. The reason I still prefer a struct type is that I've
> seen enough drivers that passed a single ad-hoc value in devtype data
> that I had to convert to pass a struct type because I had to add another
> value.
> 
> Well, I just realized that I have configured several values that I could
> have put in that struct in the SoC specific init function instead, so
> there's no real point in using a struct type.

I'm fine with the above being a struct - it was more the question of why
do everything in one 'init' function rather than having more specific
callbacks, e.g. one to just read the DDR type where you need it.

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel