2023-06-23 07:48:57

by Radu Pirea (NXP OSS)

[permalink] [raw]
Subject: [PATCH v2 03/13] net: phy: nxp-c45-tja11xx: add *_reg_field functions

Between TJA1120 and TJA1103 the hardware was improved, but some register
addresses were changed and some bit fields were moved from one register
to another.

To integrate more PHYs in the same driver with the same register fields,
but these register fields located in different registers at
different offsets, I introduced the nxp_c45_reg_field structure.

Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
---
drivers/net/phy/nxp-c45-tja11xx.c | 82 +++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)

diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index f1f15a1b6cfc..2664b3bfcb35 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -191,6 +191,21 @@ struct nxp_c45_skb_cb {
unsigned int type;
};

+#define NXP_C45_REG_FIELD(_reg, _devad, _offset, _size) \
+ ((struct nxp_c45_reg_field) { \
+ .reg = _reg, \
+ .devad = _devad, \
+ .offset = _offset, \
+ .size = _size, \
+ })
+
+struct nxp_c45_reg_field {
+ u16 reg;
+ u8 devad;
+ u8 offset;
+ u8 size;
+};
+
struct nxp_c45_hwts {
u32 nsec;
u32 sec;
@@ -225,6 +240,73 @@ struct nxp_c45_phy_stats {
u16 mask;
};

+static int nxp_c45_read_reg_field(struct phy_device *phydev,
+ const struct nxp_c45_reg_field *reg_field)
+{
+ u16 mask;
+ int ret;
+
+ if (reg_field->size == 0) {
+ phydev_err(phydev, "Trying to read a reg field of size 0.\n");
+ return -EINVAL;
+ }
+
+ ret = phy_read_mmd(phydev, reg_field->devad, reg_field->reg);
+ if (ret < 0)
+ return ret;
+
+ mask = reg_field->size == 1 ? BIT(reg_field->offset) :
+ GENMASK(reg_field->offset + reg_field->size - 1,
+ reg_field->offset);
+ ret &= mask;
+ ret >>= reg_field->offset;
+
+ return ret;
+}
+
+static int nxp_c45_write_reg_field(struct phy_device *phydev,
+ const struct nxp_c45_reg_field *reg_field,
+ u16 val)
+{
+ u16 mask;
+ u16 set;
+
+ if (reg_field->size == 0) {
+ phydev_err(phydev, "Trying to write a reg field of size 0.\n");
+ return -EINVAL;
+ }
+
+ mask = reg_field->size == 1 ? BIT(reg_field->offset) :
+ GENMASK(reg_field->offset + reg_field->size - 1,
+ reg_field->offset);
+ set = val << reg_field->offset;
+
+ return phy_modify_mmd_changed(phydev, reg_field->devad,
+ reg_field->reg, mask, set);
+}
+
+static int nxp_c45_set_reg_field(struct phy_device *phydev,
+ const struct nxp_c45_reg_field *reg_field)
+{
+ if (reg_field->size != 1) {
+ phydev_err(phydev, "Trying to set a reg field of size different than 1.\n");
+ return -EINVAL;
+ }
+
+ return nxp_c45_write_reg_field(phydev, reg_field, 1);
+}
+
+static int nxp_c45_clear_reg_field(struct phy_device *phydev,
+ const struct nxp_c45_reg_field *reg_field)
+{
+ if (reg_field->size != 1) {
+ phydev_err(phydev, "Trying to set a reg field of size different than 1.\n");
+ return -EINVAL;
+ }
+
+ return nxp_c45_write_reg_field(phydev, reg_field, 0);
+}
+
static bool nxp_c45_poll_txts(struct phy_device *phydev)
{
return phydev->irq <= 0;
--
2.34.1



2023-06-23 19:46:23

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2 03/13] net: phy: nxp-c45-tja11xx: add *_reg_field functions

On Fri, Jun 23, 2023 at 10:41:13AM +0300, Radu Pirea (NXP OSS) wrote:
> Between TJA1120 and TJA1103 the hardware was improved, but some register
> addresses were changed and some bit fields were moved from one register
> to another.
>
> To integrate more PHYs in the same driver with the same register fields,
> but these register fields located in different registers at
> different offsets, I introduced the nxp_c45_reg_field structure.
>
> Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
> ---
> drivers/net/phy/nxp-c45-tja11xx.c | 82 +++++++++++++++++++++++++++++++
> 1 file changed, 82 insertions(+)
>
> diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
> index f1f15a1b6cfc..2664b3bfcb35 100644
> --- a/drivers/net/phy/nxp-c45-tja11xx.c
> +++ b/drivers/net/phy/nxp-c45-tja11xx.c
> @@ -191,6 +191,21 @@ struct nxp_c45_skb_cb {
> unsigned int type;
> };
>
> +#define NXP_C45_REG_FIELD(_reg, _devad, _offset, _size) \
> + ((struct nxp_c45_reg_field) { \
> + .reg = _reg, \
> + .devad = _devad, \
> + .offset = _offset, \
> + .size = _size, \
> + })
> +
> +struct nxp_c45_reg_field {
> + u16 reg;
> + u8 devad;
> + u8 offset;
> + u8 size;
> +};
> +
> struct nxp_c45_hwts {
> u32 nsec;
> u32 sec;
> @@ -225,6 +240,73 @@ struct nxp_c45_phy_stats {
> u16 mask;
> };
>
> +static int nxp_c45_read_reg_field(struct phy_device *phydev,
> + const struct nxp_c45_reg_field *reg_field)

Hi Radu,

I think this is resolved in a latter patch in this series.
But nxp_c45_read_reg_field is defined but not used.
As is nxp_c45_set_reg_field and nxp_c45_clear_reg_field.

This causes an allmodconfig build to fail on x86_64 with GCC 12.3.0.

--
pw-bot: changes-requested


2023-06-26 09:03:20

by Radu Pirea (NXP OSS)

[permalink] [raw]
Subject: Re: [PATCH v2 03/13] net: phy: nxp-c45-tja11xx: add *_reg_field functions

On 23.06.2023 22:32, Simon Horman wrote:
> On Fri, Jun 23, 2023 at 10:41:13AM +0300, Radu Pirea (NXP OSS) wrote:
>> Between TJA1120 and TJA1103 the hardware was improved, but some register
>> addresses were changed and some bit fields were moved from one register
>> to another.
>>
>> To integrate more PHYs in the same driver with the same register fields,
>> but these register fields located in different registers at
>> different offsets, I introduced the nxp_c45_reg_field structure.
>>
>> Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
>> ---
>> drivers/net/phy/nxp-c45-tja11xx.c | 82 +++++++++++++++++++++++++++++++
>> 1 file changed, 82 insertions(+)
>>
>> diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
>> index f1f15a1b6cfc..2664b3bfcb35 100644
>> --- a/drivers/net/phy/nxp-c45-tja11xx.c
>> +++ b/drivers/net/phy/nxp-c45-tja11xx.c
>> @@ -191,6 +191,21 @@ struct nxp_c45_skb_cb {
>> unsigned int type;
>> };
>>
>> +#define NXP_C45_REG_FIELD(_reg, _devad, _offset, _size) \
>> + ((struct nxp_c45_reg_field) { \
>> + .reg = _reg, \
>> + .devad = _devad, \
>> + .offset = _offset, \
>> + .size = _size, \
>> + })
>> +
>> +struct nxp_c45_reg_field {
>> + u16 reg;
>> + u8 devad;
>> + u8 offset;
>> + u8 size;
>> +};
>> +
>> struct nxp_c45_hwts {
>> u32 nsec;
>> u32 sec;
>> @@ -225,6 +240,73 @@ struct nxp_c45_phy_stats {
>> u16 mask;
>> };
>>
>> +static int nxp_c45_read_reg_field(struct phy_device *phydev,
>> + const struct nxp_c45_reg_field *reg_field)
>
> Hi Radu,
>
> I think this is resolved in a latter patch in this series.
> But nxp_c45_read_reg_field is defined but not used.
> As is nxp_c45_set_reg_field and nxp_c45_clear_reg_field.
>
> This causes an allmodconfig build to fail on x86_64 with GCC 12.3.0.
>
> --
> pw-bot: changes-requested
>

I will merge the commits 3 and 4.
Thank you.

--
Radu P.