Hi Marek,
Am Dienstag, 3. Januar 2023, 15:31:21 CET schrieb Marek Vasut:
> On 1/3/23 13:31, Alexander Stein wrote:
> > The calculation DIFx is BIT(n) +1 is only true for 9FGV0241. With
> > additional devices this is getting more complicated.
> > Support a base bit for the DIF calculation, currently only devices
> > with consecutive bits are supported, e.g. the 6-channel device needs
> > additional logic.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> > ---
> >
> > drivers/clk/clk-renesas-pcie.c | 29 ++++++++++++++++-------------
> > 1 file changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/clk/clk-renesas-pcie.c
> > b/drivers/clk/clk-renesas-pcie.c index 0076ed8f11b0..d19b8e759eea 100644
> > --- a/drivers/clk/clk-renesas-pcie.c
> > +++ b/drivers/clk/clk-renesas-pcie.c
> > @@ -18,7 +18,6 @@
> >
> > #include <linux/regmap.h>
> >
> > #define RS9_REG_OE 0x0
> >
> > -#define RS9_REG_OE_DIF_OE(n) BIT((n) + 1)
> >
> > #define RS9_REG_SS 0x1
> > #define RS9_REG_SS_AMP_0V6 0x0
> > #define RS9_REG_SS_AMP_0V7 0x1
> >
> > @@ -31,9 +30,6 @@
> >
> > #define RS9_REG_SS_SSC_MASK (3 << 3)
> > #define RS9_REG_SS_SSC_LOCK BIT(5)
> > #define RS9_REG_SR 0x2
> >
> > -#define RS9_REG_SR_2V0_DIF(n) 0
> > -#define RS9_REG_SR_3V0_DIF(n) BIT((n) + 1)
> > -#define RS9_REG_SR_DIF_MASK(n) BIT((n) + 1)
> >
> > #define RS9_REG_REF 0x3
> > #define RS9_REG_REF_OE BIT(4)
> > #define RS9_REG_REF_OD BIT(5)
> >
> > @@ -62,6 +58,7 @@ struct rs9_chip_info {
> >
> > const enum rs9_model model;
> > unsigned int num_clks;
> > u8 did;
> >
> > + u8 (*calc_dif)(int idx);
> >
> > };
> >
> > struct rs9_driver_data {
> >
> > @@ -160,8 +157,14 @@ static const struct regmap_config rs9_regmap_config =
> > {>
> > .reg_read = rs9_regmap_i2c_read,
> >
> > };
> >
> > +static u8 rs9fgv0241_calc_dif(int idx)
> > +{
> > + return BIT(idx) + 1;
>
> Can't we just do
>
> if (model == ...)
> return BIT(idx) + 1
> else if (model == ...)
> return BIT(idx);
> ...
I was tempted going this way. But I opted for a callback due to the fact that
this driver might support 9FGV/9DBV/9DMV/9FGL/9DML/9QXL/9SQ as well(your
comment in the header).
Even just considering 9FVG, 9FGV0641 has an even more complex DIF offset
calculation.
The mapping is
* DIF OE0 - Bit 0
* DIF OE1 - Bit 2
* DIF OE2 - Bit 3
* DIF OE3 - Bit 4
* DIF OE4 - Bit 6
* DIF OE5 - Bit 7
So the calucation might not fit into one line, so the readability benefit is
gone.
Best regards,
Alexander
