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@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.
> + */
> 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);
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
