* [PATCH v2 1/4] clk: rs9: Check for vendor/device ID
@ 2023-01-10 10:00 Alexander Stein
2023-01-10 10:00 ` [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441 Alexander Stein
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Alexander Stein @ 2023-01-10 10:00 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Marek Vasut
Cc: Alexander Stein, linux-renesas-soc, linux-clk, devicetree
This is in preparation to support additional devices which have different
IDs as well as a slightly different register layout.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Use dev_err_probe to include return statement in one line
drivers/clk/clk-renesas-pcie.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
index e6247141d0c05..bba09a88c2ccc 100644
--- a/drivers/clk/clk-renesas-pcie.c
+++ b/drivers/clk/clk-renesas-pcie.c
@@ -45,6 +45,13 @@
#define RS9_REG_DID 0x6
#define RS9_REG_BCP 0x7
+#define RS9_REG_VID_IDT 0x01
+
+#define RS9_REG_DID_TYPE_FGV (0x0 << RS9_REG_DID_TYPE_SHIFT)
+#define RS9_REG_DID_TYPE_DBV (0x1 << RS9_REG_DID_TYPE_SHIFT)
+#define RS9_REG_DID_TYPE_DMV (0x2 << RS9_REG_DID_TYPE_SHIFT)
+#define RS9_REG_DID_TYPE_SHIFT 0x6
+
/* Supported Renesas 9-series models. */
enum rs9_model {
RENESAS_9FGV0241,
@@ -54,6 +61,7 @@ enum rs9_model {
struct rs9_chip_info {
const enum rs9_model model;
unsigned int num_clks;
+ u8 did;
};
struct rs9_driver_data {
@@ -270,6 +278,7 @@ static int rs9_probe(struct i2c_client *client)
{
unsigned char name[5] = "DIF0";
struct rs9_driver_data *rs9;
+ unsigned int vid, did;
struct clk_hw *hw;
int i, ret;
@@ -306,6 +315,20 @@ static int rs9_probe(struct i2c_client *client)
if (ret < 0)
return ret;
+ ret = regmap_read(rs9->regmap, RS9_REG_VID, &vid);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(rs9->regmap, RS9_REG_DID, &did);
+ if (ret < 0)
+ return ret;
+
+ 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",
+ vid, did, RS9_REG_VID_IDT,
+ rs9->chip_info->did);
+
/* Register clock */
for (i = 0; i < rs9->chip_info->num_clks; i++) {
snprintf(name, 5, "DIF%d", i);
@@ -349,6 +372,7 @@ static int __maybe_unused rs9_resume(struct device *dev)
static const struct rs9_chip_info renesas_9fgv0241_info = {
.model = RENESAS_9FGV0241,
.num_clks = 2,
+ .did = RS9_REG_DID_TYPE_FGV | 0x02,
};
static const struct i2c_device_id rs9_id[] = {
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441
2023-01-10 10:00 [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Alexander Stein
@ 2023-01-10 10:00 ` Alexander Stein
2023-01-10 10:29 ` Marek Vasut
2023-01-10 10:00 ` [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation Alexander Stein
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Alexander Stein @ 2023-01-10 10:00 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Marek Vasut
Cc: Alexander Stein, linux-renesas-soc, linux-clk, devicetree,
Krzysztof Kozlowski
This is a 4-channel variant of 9FGV series.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
* Add Krzysztof's A-b
.../devicetree/bindings/clock/renesas,9series.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/renesas,9series.yaml b/Documentation/devicetree/bindings/clock/renesas,9series.yaml
index 6b6cec3fba528..3afdebdb52ad4 100644
--- a/Documentation/devicetree/bindings/clock/renesas,9series.yaml
+++ b/Documentation/devicetree/bindings/clock/renesas,9series.yaml
@@ -16,6 +16,11 @@ description: |
- 9FGV0241:
0 -- DIF0
1 -- DIF1
+ - 9FGV0441:
+ 0 -- DIF0
+ 1 -- DIF1
+ 2 -- DIF2
+ 3 -- DIF3
maintainers:
- Marek Vasut <marex@denx.de>
@@ -24,6 +29,7 @@ properties:
compatible:
enum:
- renesas,9fgv0241
+ - renesas,9fgv0441
reg:
description: I2C device address
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation
2023-01-10 10:00 [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Alexander Stein
2023-01-10 10:00 ` [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441 Alexander Stein
@ 2023-01-10 10:00 ` Alexander Stein
2023-01-10 10:31 ` Marek Vasut
2023-01-10 10:00 ` [PATCH v2 4/4] clk: rs9: Add support for 9FGV0441 Alexander Stein
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Alexander Stein @ 2023-01-10 10:00 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Marek Vasut
Cc: Alexander Stein, linux-renesas-soc, linux-clk, devicetree
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@ew.tq-group.com>
---
Changes in v2:
* Use a common function instead of callback for calculating the DIF bit
drivers/clk/clk-renesas-pcie.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
index bba09a88c2ccc..6b19186228238 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)
@@ -160,17 +156,27 @@ static const struct regmap_config rs9_regmap_config = {
.reg_read = rs9_regmap_i2c_read,
};
+static u8 rs9_calc_dif(const struct rs9_driver_data *rs9, int idx)
+{
+ enum rs9_model model = rs9->chip_info->model;
+
+ if (model == RENESAS_9FGV0241)
+ return BIT(idx) + 1;
+
+ return 0;
+}
+
static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
{
struct i2c_client *client = rs9->client;
+ u8 dif = rs9_calc_dif(rs9, idx);
unsigned char name[5] = "DIF0";
struct device_node *np;
int ret;
u32 sr;
/* Set defaults */
- rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
- rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx);
+ rs9->clk_dif_sr |= dif;
snprintf(name, 5, "DIF%d", idx);
np = of_get_child_by_name(client->dev.of_node, name);
@@ -182,11 +188,9 @@ static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
of_node_put(np);
if (!ret) {
if (sr == 2000000) { /* 2V/ns */
- rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
- rs9->clk_dif_sr |= RS9_REG_SR_2V0_DIF(idx);
+ rs9->clk_dif_sr &= ~dif;
} else if (sr == 3000000) { /* 3V/ns (default) */
- rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
- rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx);
+ rs9->clk_dif_sr |= dif;
} else
ret = dev_err_probe(&client->dev, -EINVAL,
"Invalid renesas,slew-rate value\n");
@@ -257,11 +261,13 @@ static void rs9_update_config(struct rs9_driver_data *rs9)
}
for (i = 0; i < rs9->chip_info->num_clks; i++) {
- if (rs9->clk_dif_sr & RS9_REG_SR_3V0_DIF(i))
+ u8 dif = rs9_calc_dif(rs9, i);
+
+ if (rs9->clk_dif_sr & dif)
continue;
- regmap_update_bits(rs9->regmap, RS9_REG_SR, RS9_REG_SR_3V0_DIF(i),
- rs9->clk_dif_sr & RS9_REG_SR_3V0_DIF(i));
+ regmap_update_bits(rs9->regmap, RS9_REG_SR, dif,
+ rs9->clk_dif_sr & dif);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] clk: rs9: Add support for 9FGV0441
2023-01-10 10:00 [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Alexander Stein
2023-01-10 10:00 ` [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441 Alexander Stein
2023-01-10 10:00 ` [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation Alexander Stein
@ 2023-01-10 10:00 ` Alexander Stein
2023-01-10 10:32 ` Marek Vasut
2023-01-10 10:28 ` [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Marek Vasut
2023-02-13 7:28 ` Alexander Stein
4 siblings, 1 reply; 13+ messages in thread
From: Alexander Stein @ 2023-01-10 10:00 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Marek Vasut
Cc: Alexander Stein, linux-renesas-soc, linux-clk, devicetree
This model is similar to 9FGV0241, but the DIFx bits start at bit 0.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Adjust to common DIF calculation function
drivers/clk/clk-renesas-pcie.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
index 6b19186228238..a4ebb224181bf 100644
--- a/drivers/clk/clk-renesas-pcie.c
+++ b/drivers/clk/clk-renesas-pcie.c
@@ -6,6 +6,7 @@
* - 9FGV/9DBV/9DMV/9FGL/9DML/9QXL/9SQ
* Currently supported:
* - 9FGV0241
+ * - 9FGV0441
*
* Copyright (C) 2022 Marek Vasut <marex@denx.de>
*/
@@ -51,6 +52,7 @@
/* Supported Renesas 9-series models. */
enum rs9_model {
RENESAS_9FGV0241,
+ RENESAS_9FGV0441,
};
/* Structure to describe features of a particular 9-series model */
@@ -65,7 +67,7 @@ struct rs9_driver_data {
struct regmap *regmap;
const struct rs9_chip_info *chip_info;
struct clk *pin_xin;
- struct clk_hw *clk_dif[2];
+ struct clk_hw *clk_dif[4];
u8 pll_amplitude;
u8 pll_ssc;
u8 clk_dif_sr;
@@ -162,6 +164,8 @@ static u8 rs9_calc_dif(const struct rs9_driver_data *rs9, int idx)
if (model == RENESAS_9FGV0241)
return BIT(idx) + 1;
+ else if (model == RENESAS_9FGV0441)
+ return BIT(idx);
return 0;
}
@@ -381,14 +385,22 @@ static const struct rs9_chip_info renesas_9fgv0241_info = {
.did = RS9_REG_DID_TYPE_FGV | 0x02,
};
+static const struct rs9_chip_info renesas_9fgv0441_info = {
+ .model = RENESAS_9FGV0441,
+ .num_clks = 4,
+ .did = RS9_REG_DID_TYPE_FGV | 0x04,
+};
+
static const struct i2c_device_id rs9_id[] = {
{ "9fgv0241", .driver_data = RENESAS_9FGV0241 },
+ { "9fgv0441", .driver_data = RENESAS_9FGV0441 },
{ }
};
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 },
{ }
};
MODULE_DEVICE_TABLE(of, clk_rs9_of_match);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] clk: rs9: Check for vendor/device ID
2023-01-10 10:00 [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Alexander Stein
` (2 preceding siblings ...)
2023-01-10 10:00 ` [PATCH v2 4/4] clk: rs9: Add support for 9FGV0441 Alexander Stein
@ 2023-01-10 10:28 ` Marek Vasut
2023-02-13 7:28 ` Alexander Stein
4 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2023-01-10 10:28 UTC (permalink / raw)
To: Alexander Stein, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski
Cc: linux-renesas-soc, linux-clk, devicetree
On 1/10/23 11:00, Alexander Stein wrote:
> This is in preparation to support additional devices which have different
> IDs as well as a slightly different register layout.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Marek Vasut <marex@denx.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441
2023-01-10 10:00 ` [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441 Alexander Stein
@ 2023-01-10 10:29 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2023-01-10 10:29 UTC (permalink / raw)
To: Alexander Stein, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski
Cc: linux-renesas-soc, linux-clk, devicetree, Krzysztof Kozlowski
On 1/10/23 11:00, Alexander Stein wrote:
> This is a 4-channel variant of 9FGV series.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Marek Vasut <marex@denx.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation
2023-01-10 10:00 ` [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation Alexander Stein
@ 2023-01-10 10:31 ` Marek Vasut
2023-01-10 13:22 ` Alexander Stein
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2023-01-10 10:31 UTC (permalink / raw)
To: Alexander Stein, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski
Cc: linux-renesas-soc, linux-clk, devicetree
On 1/10/23 11:00, Alexander Stein wrote:
[...]
> static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
> {
> struct i2c_client *client = rs9->client;
> + u8 dif = rs9_calc_dif(rs9, idx);
> unsigned char name[5] = "DIF0";
> struct device_node *np;
> int ret;
> u32 sr;
>
> /* Set defaults */
> - rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
Are you sure this line ^ should be dropped ?
Shouldn't the bitfield be cleared first and modified second?
> - rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx);
> + rs9->clk_dif_sr |= dif;
>
> snprintf(name, 5, "DIF%d", idx);
> np = of_get_child_by_name(client->dev.of_node, name);
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] clk: rs9: Add support for 9FGV0441
2023-01-10 10:00 ` [PATCH v2 4/4] clk: rs9: Add support for 9FGV0441 Alexander Stein
@ 2023-01-10 10:32 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2023-01-10 10:32 UTC (permalink / raw)
To: Alexander Stein, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski
Cc: linux-renesas-soc, linux-clk, devicetree
On 1/10/23 11:00, Alexander Stein wrote:
> This model is similar to 9FGV0241, but the DIFx bits start at bit 0.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Marek Vasut <marex@denx.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation
2023-01-10 10:31 ` Marek Vasut
@ 2023-01-10 13:22 ` Alexander Stein
2023-01-10 13:37 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Stein @ 2023-01-10 13:22 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Marek Vasut
Cc: linux-renesas-soc, linux-clk, devicetree
Hi Marek,
thanks for your feedback.
Am Dienstag, 10. Januar 2023, 11:31:49 CET schrieb Marek Vasut:
> On 1/10/23 11:00, Alexander Stein wrote:
>
> [...]
>
> > static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
> > {
> >
> > struct i2c_client *client = rs9->client;
> >
> > + u8 dif = rs9_calc_dif(rs9, idx);
> >
> > unsigned char name[5] = "DIF0";
> > struct device_node *np;
> > int ret;
> > u32 sr;
> >
> > /* Set defaults */
> >
> > - rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
>
> Are you sure this line ^ should be dropped ?
> Shouldn't the bitfield be cleared first and modified second?
Well, I had in my mind that this function is called upon probe with clk_dif_sr
being cleared anyway, so this does essentially nothing. And the DIF bit is set
unconditionally, so what is the point of masking it before?
Best regards,
Alexander
> > - rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx);
> > + rs9->clk_dif_sr |= dif;
> >
> > snprintf(name, 5, "DIF%d", idx);
> > np = of_get_child_by_name(client->dev.of_node, name);
>
> [...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation
2023-01-10 13:22 ` Alexander Stein
@ 2023-01-10 13:37 ` Marek Vasut
2023-01-10 13:47 ` Alexander Stein
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2023-01-10 13:37 UTC (permalink / raw)
To: Alexander Stein, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski
Cc: linux-renesas-soc, linux-clk, devicetree
On 1/10/23 14:22, Alexander Stein wrote:
> Hi Marek,
Hi,
> thanks for your feedback.
>
> Am Dienstag, 10. Januar 2023, 11:31:49 CET schrieb Marek Vasut:
>> On 1/10/23 11:00, Alexander Stein wrote:
>>
>> [...]
>>
>>> static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
>>> {
>>>
>>> struct i2c_client *client = rs9->client;
>>>
>>> + u8 dif = rs9_calc_dif(rs9, idx);
>>>
>>> unsigned char name[5] = "DIF0";
>>> struct device_node *np;
>>> int ret;
>>> u32 sr;
>>>
>>> /* Set defaults */
>>>
>>> - rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
>>
>> Are you sure this line ^ should be dropped ?
>> Shouldn't the bitfield be cleared first and modified second?
>
> Well, I had in my mind that this function is called upon probe with clk_dif_sr
> being cleared anyway, so this does essentially nothing. And the DIF bit is set
> unconditionally, so what is the point of masking it before?
Good point, but then, what's the point of ORRing either ? Just do a
plain assignment.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation
2023-01-10 13:37 ` Marek Vasut
@ 2023-01-10 13:47 ` Alexander Stein
2023-01-10 13:51 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Stein @ 2023-01-10 13:47 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Marek Vasut
Cc: linux-renesas-soc, linux-clk, devicetree
Hello Marek,
Am Dienstag, 10. Januar 2023, 14:37:19 CET schrieb Marek Vasut:
> On 1/10/23 14:22, Alexander Stein wrote:
> > Hi Marek,
>
> Hi,
>
> > thanks for your feedback.
> >
> > Am Dienstag, 10. Januar 2023, 11:31:49 CET schrieb Marek Vasut:
> >> On 1/10/23 11:00, Alexander Stein wrote:
> >>
> >> [...]
> >>
> >>> static int rs9_get_output_config(struct rs9_driver_data *rs9, int
> >>> idx)
> >>> {
> >>>
> >>> struct i2c_client *client = rs9->client;
> >>>
> >>> + u8 dif = rs9_calc_dif(rs9, idx);
> >>>
> >>> unsigned char name[5] = "DIF0";
> >>> struct device_node *np;
> >>> int ret;
> >>> u32 sr;
> >>>
> >>> /* Set defaults */
> >>>
> >>> - rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
> >>
> >> Are you sure this line ^ should be dropped ?
> >> Shouldn't the bitfield be cleared first and modified second?
> >
> > Well, I had in my mind that this function is called upon probe with
> > clk_dif_sr being cleared anyway, so this does essentially nothing. And
> > the DIF bit is set unconditionally, so what is the point of masking it
> > before?
>
> Good point, but then, what's the point of ORRing either ? Just do a
> plain assignment.
OR-ring is necessary as this function is called for each DIF output (see idx
parameter), so plain assignment will clear the previously set bits.
Best regards,
Alexander
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation
2023-01-10 13:47 ` Alexander Stein
@ 2023-01-10 13:51 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2023-01-10 13:51 UTC (permalink / raw)
To: Alexander Stein, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski
Cc: linux-renesas-soc, linux-clk, devicetree
On 1/10/23 14:47, Alexander Stein wrote:
> Hello Marek,
>
> Am Dienstag, 10. Januar 2023, 14:37:19 CET schrieb Marek Vasut:
>> On 1/10/23 14:22, Alexander Stein wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> thanks for your feedback.
>>>
>>> Am Dienstag, 10. Januar 2023, 11:31:49 CET schrieb Marek Vasut:
>>>> On 1/10/23 11:00, Alexander Stein wrote:
>>>>
>>>> [...]
>>>>
>>>>> static int rs9_get_output_config(struct rs9_driver_data *rs9, int
>>>>> idx)
>>>>> {
>>>>>
>>>>> struct i2c_client *client = rs9->client;
>>>>>
>>>>> + u8 dif = rs9_calc_dif(rs9, idx);
>>>>>
>>>>> unsigned char name[5] = "DIF0";
>>>>> struct device_node *np;
>>>>> int ret;
>>>>> u32 sr;
>>>>>
>>>>> /* Set defaults */
>>>>>
>>>>> - rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
>>>>
>>>> Are you sure this line ^ should be dropped ?
>>>> Shouldn't the bitfield be cleared first and modified second?
>>>
>>> Well, I had in my mind that this function is called upon probe with
>>> clk_dif_sr being cleared anyway, so this does essentially nothing. And
>>> the DIF bit is set unconditionally, so what is the point of masking it
>>> before?
>>
>> Good point, but then, what's the point of ORRing either ? Just do a
>> plain assignment.
>
> OR-ring is necessary as this function is called for each DIF output (see idx
> parameter), so plain assignment will clear the previously set bits.
Ah, got it.
Reviewed-by: Marek Vasut <marex@denx.de>
Thanks !
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] clk: rs9: Check for vendor/device ID
2023-01-10 10:00 [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Alexander Stein
` (3 preceding siblings ...)
2023-01-10 10:28 ` [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Marek Vasut
@ 2023-02-13 7:28 ` Alexander Stein
4 siblings, 0 replies; 13+ messages in thread
From: Alexander Stein @ 2023-02-13 7:28 UTC (permalink / raw)
To: Stephen Boyd
Cc: Geert Uytterhoeven, Michael Turquette, Rob Herring,
Krzysztof Kozlowski, Marek Vasut, linux-renesas-soc, linux-clk,
devicetree
Hi Stephen,
want me to resend adding all Reviewed-By?
Best regards,
Alexander
Am Dienstag, 10. Januar 2023, 11:00:00 CET schrieb Alexander Stein:
> This is in preparation to support additional devices which have different
> IDs as well as a slightly different register layout.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * Use dev_err_probe to include return statement in one line
>
> drivers/clk/clk-renesas-pcie.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/clk/clk-renesas-pcie.c b/drivers/clk/clk-renesas-pcie.c
> index e6247141d0c05..bba09a88c2ccc 100644
> --- a/drivers/clk/clk-renesas-pcie.c
> +++ b/drivers/clk/clk-renesas-pcie.c
> @@ -45,6 +45,13 @@
> #define RS9_REG_DID 0x6
> #define RS9_REG_BCP 0x7
>
> +#define RS9_REG_VID_IDT 0x01
> +
> +#define RS9_REG_DID_TYPE_FGV (0x0 <<
RS9_REG_DID_TYPE_SHIFT)
> +#define RS9_REG_DID_TYPE_DBV (0x1 <<
RS9_REG_DID_TYPE_SHIFT)
> +#define RS9_REG_DID_TYPE_DMV (0x2 <<
RS9_REG_DID_TYPE_SHIFT)
> +#define RS9_REG_DID_TYPE_SHIFT 0x6
> +
> /* Supported Renesas 9-series models. */
> enum rs9_model {
> RENESAS_9FGV0241,
> @@ -54,6 +61,7 @@ enum rs9_model {
> struct rs9_chip_info {
> const enum rs9_model model;
> unsigned int num_clks;
> + u8 did;
> };
>
> struct rs9_driver_data {
> @@ -270,6 +278,7 @@ static int rs9_probe(struct i2c_client *client)
> {
> unsigned char name[5] = "DIF0";
> struct rs9_driver_data *rs9;
> + unsigned int vid, did;
> struct clk_hw *hw;
> int i, ret;
>
> @@ -306,6 +315,20 @@ static int rs9_probe(struct i2c_client *client)
> if (ret < 0)
> return ret;
>
> + ret = regmap_read(rs9->regmap, RS9_REG_VID, &vid);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(rs9->regmap, RS9_REG_DID, &did);
> + if (ret < 0)
> + return ret;
> +
> + 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",
> + vid, did, RS9_REG_VID_IDT,
> + rs9->chip_info->did);
> +
> /* Register clock */
> for (i = 0; i < rs9->chip_info->num_clks; i++) {
> snprintf(name, 5, "DIF%d", i);
> @@ -349,6 +372,7 @@ static int __maybe_unused rs9_resume(struct device *dev)
> static const struct rs9_chip_info renesas_9fgv0241_info = {
> .model = RENESAS_9FGV0241,
> .num_clks = 2,
> + .did = RS9_REG_DID_TYPE_FGV | 0x02,
> };
>
> static const struct i2c_device_id rs9_id[] = {
--
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/
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-02-13 7:28 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-10 10:00 [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Alexander Stein
2023-01-10 10:00 ` [PATCH v2 2/4] dt-bindings: clk: rs9: Add 9FGV0441 Alexander Stein
2023-01-10 10:29 ` Marek Vasut
2023-01-10 10:00 ` [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation Alexander Stein
2023-01-10 10:31 ` Marek Vasut
2023-01-10 13:22 ` Alexander Stein
2023-01-10 13:37 ` Marek Vasut
2023-01-10 13:47 ` Alexander Stein
2023-01-10 13:51 ` Marek Vasut
2023-01-10 10:00 ` [PATCH v2 4/4] clk: rs9: Add support for 9FGV0441 Alexander Stein
2023-01-10 10:32 ` Marek Vasut
2023-01-10 10:28 ` [PATCH v2 1/4] clk: rs9: Check for vendor/device ID Marek Vasut
2023-02-13 7:28 ` Alexander Stein
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.