RE: [PATCH 2/2] clk: rs9: Add support for 9FGV0841

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alexander Stein,

> -----Original Message-----
> From: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH 2/2] clk: rs9: Add support for 9FGV0841
> 
> Hi,
> 
> Am Montag, 6. November 2023, 08:49:10 CET schrieb Biju Das:
> > Hi Marek,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Marek Vasut <marek.vasut+renesas@xxxxxxxxxxx>
> > > Sent: Sunday, November 5, 2023 8:08 PM
> > > Subject: [PATCH 2/2] clk: rs9: Add support for 9FGV0841
> > >
> > > This model is similar to 9FGV0441, the DIFx bits start at bit 0
> > > again, except this chip has 8 outputs. Adjust rs9_calc_dif() to
> > > special-case the
> > > 9FGV0241 where DIFx bits start at 1. Extract only vendor ID from VID
> > > register, the top 4 bits are revision ID which are not useful for
> > > the vendor ID check.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxxxx>
> > > ---
> > > Cc: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> > > Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
> > > Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
> > > Cc: Michael Turquette <mturquette@xxxxxxxxxxxx>
> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > > Cc: Stephen Boyd <sboyd@xxxxxxxxxx>
> > > Cc: devicetree@xxxxxxxxxxxxxxx
> > > Cc: linux-clk@xxxxxxxxxxxxxxx
> > > Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
> > > ---
> > >
> > >  drivers/clk/clk-renesas-pcie.c | 23 +++++++++++++++++++----
> > >  1 file changed, 19 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk-renesas-pcie.c
> > > b/drivers/clk/clk-renesas- pcie.c index 6606aba253c5..f8dd79b18d5a
> > > 100644
> > > --- a/drivers/clk/clk-renesas-pcie.c
> > > +++ b/drivers/clk/clk-renesas-pcie.c
> > > @@ -7,6 +7,7 @@
> > >
> > >   * Currently supported:
> > >   *   - 9FGV0241
> > >   *   - 9FGV0441
> > >
> > > + *   - 9FGV0841
> > >
> > >   *
> > >   * Copyright (C) 2022 Marek Vasut <marex@xxxxxxx>
> > >   */
> > >
> > > @@ -42,6 +43,7 @@
> > >
> > >  #define RS9_REG_DID				0x6
> > >  #define RS9_REG_BCP				0x7
> > >
> > > +#define RS9_REG_VID_MASK			GENMASK(3, 0)
> > >
> > >  #define RS9_REG_VID_IDT				0x01
> > >
> > >  #define RS9_REG_DID_TYPE_FGV			(0x0 <<
> RS9_REG_DID_TYPE_SHIFT)
> > >
> > > @@ -53,6 +55,7 @@
> > >
> > >  enum rs9_model {
> > >
> > >  	RENESAS_9FGV0241,
> > >  	RENESAS_9FGV0441,
> > >
> > > +	RENESAS_9FGV0841,
> > >
> > >  };
> > >
> > >  /* Structure to describe features of a particular 9-series model */
> > > @@ -
> > >
> > > 162,12 +165,15 @@ static u8 rs9_calc_dif(const struct
> > > rs9_driver_data *rs9, int idx)  {
> > >
> > >  	enum rs9_model model = rs9->chip_info->model;
> > >
> > > +	/*
> > > +	 * On 9FGV0241, the DIF OE0 is BIT(1) and DIF OE(1) is BIT(2),
> > > +	 * on 9FGV0441 and 9FGV0841 the DIF OE0 is BIT(0) and so on.
> > > +	 * Increment the index in the 9FGV0241 special case here.
> > > +	 */
> >
> > I guess model enum variable in struct rs9_chip_info can be replaced
> > with a variable for the above hardware differences(eg: BIT(idx) value
> > in struct
> > rs9_chip_inf) . Then you don't need this function at all.
> 
> That's true for 9FGV family. If support for 9QXL family will ever be added
> (the header claims the support can be added), this enum is required again
> as the register model is completely different.


I may be wrong, Still all the hw differences can be handled by feature, data and functions in
struct rs9_chip_info. You don't need comparison with model all over the places.

Cheers,
Biju


> 
> Best regards,
> Alexander
> 
> > Cheers,
> > Biju
> >
> > >  	if (model == RENESAS_9FGV0241)
> > >
> > > -		return BIT(idx + 1);
> > > -	else if (model == RENESAS_9FGV0441)
> > > -		return BIT(idx);
> > > +		idx++;
> > >
> > > -	return 0;
> > > +	return BIT(idx);
> > >
> > >  }
> > >
> > >  static int rs9_get_output_config(struct rs9_driver_data *rs9, int
> > > idx) @@
> > >
> > > -333,6 +339,7 @@ static int rs9_probe(struct i2c_client *client)
> > >
> > >  	if (ret < 0)
> > >
> > >  		return ret;
> > >
> > > +	vid &= RS9_REG_VID_MASK;
> > >
> > >  	if (vid != RS9_REG_VID_IDT || did != rs9->chip_info->did)
> > >
> > >  		return dev_err_probe(&client->dev, -ENODEV,
> > >
> > >  				     "Incorrect VID/DID: %#02x, %#02x.
> > >
> > > Expected %#02x, %#02x\n", @@ -391,9 +398,16 @@ static const struct
> > > rs9_chip_info renesas_9fgv0441_info = {
> > >
> > >  	.did		= RS9_REG_DID_TYPE_FGV | 0x04,
> > >
> > >  };
> > >
> > > +static const struct rs9_chip_info renesas_9fgv0841_info = {
> > > +	.model		= RENESAS_9FGV0841,
> > > +	.num_clks	= 8,
> > > +	.did		= RS9_REG_DID_TYPE_FGV | 0x08,
> > > +};
> > > +
> > >
> > >  static const struct i2c_device_id rs9_id[] = {
> > >
> > >  	{ "9fgv0241", .driver_data =
> > >
> > > (kernel_ulong_t)&renesas_9fgv0241_info },
> > >
> > >  	{ "9fgv0441", .driver_data =
> > >
> > > (kernel_ulong_t)&renesas_9fgv0441_info },
> > > +	{ "9fgv0841", .driver_data =
> > > (kernel_ulong_t)&renesas_9fgv0841_info },
> > >
> > >  	{ }
> > >
> > >  };
> > >  MODULE_DEVICE_TABLE(i2c, rs9_id);
> > >
> > > @@ -401,6 +415,7 @@ MODULE_DEVICE_TABLE(i2c, rs9_id);  static const
> > > struct of_device_id clk_rs9_of_match[] = {
> > >
> > >  	{ .compatible = "renesas,9fgv0241", .data =
> > >
> > > &renesas_9fgv0241_info },
> > >
> > >  	{ .compatible = "renesas,9fgv0441", .data =
> > >
> > > &renesas_9fgv0441_info },
> > > +	{ .compatible = "renesas,9fgv0841", .data =
> > > &renesas_9fgv0841_info },
> > >
> > >  	{ }
> > >
> > >  };
> > >  MODULE_DEVICE_TABLE(of, clk_rs9_of_match);
> > >
> > > --
> > > 2.42.0
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux