diff mbox series

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

Message ID 20231105200812.62849-2-marek.vasut+renesas@mailbox.org (mailing list archive)
State Changes Requested, archived
Headers show
Series [1/2] dt-bindings: clk: rs9: Add 9FGV0841 | expand

Commit Message

Marek Vasut Nov. 5, 2023, 8:07 p.m. UTC
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@mailbox.org>
---
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/clk/clk-renesas-pcie.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

Comments

Biju Das Nov. 6, 2023, 7:49 a.m. UTC | #1
Hi Marek,

Thanks for the patch.

> -----Original Message-----
> From: Marek Vasut <marek.vasut+renesas@mailbox.org>
> 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@mailbox.org>
> ---
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  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@denx.de>
>   */
> @@ -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.

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
Alexander Stein Nov. 6, 2023, 10 a.m. UTC | #2
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@mailbox.org>
> > 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@mailbox.org>
> > ---
> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Cc: Conor Dooley <conor+dt@kernel.org>
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-clk@vger.kernel.org
> > Cc: linux-renesas-soc@vger.kernel.org
> > ---
> > 
> >  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@denx.de>
> >   */
> > 
> > @@ -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.

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
Biju Das Nov. 6, 2023, 10:08 a.m. UTC | #3
Hi Alexander Stein,

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> 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@mailbox.org>
> > > 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@mailbox.org>
> > > ---
> > > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > Cc: Conor Dooley <conor+dt@kernel.org>
> > > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: linux-clk@vger.kernel.org
> > > Cc: linux-renesas-soc@vger.kernel.org
> > > ---
> > >
> > >  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@denx.de>
> > >   */
> > >
> > > @@ -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
>
Alexander Stein Nov. 6, 2023, 10:15 a.m. UTC | #4
Hi Marek,

thanks for the patch.

Am Sonntag, 5. November 2023, 21:07:59 CET schrieb Marek Vasut:
> 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@mailbox.org>
> ---
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  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@denx.de>
>   */
> @@ -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.
> +	 */
>  	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;

This is an independent change from 9FGV0841 support. So please split it into a 
separate patch.
Despite that adding the support looks good. I don't have a strong opinion 
regarding the model check as raised by Biju.

Best regards,
Alexander

>  	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);
diff mbox series

Patch

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@denx.de>
  */
@@ -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.
+	 */
 	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);