Generic MDIO-device driver will support setting a custom device ID for the
particular MDIO-device. Generic DW XPCS ID can be used for that as well
just in case if the DW XPCS-device was erroneously synthesized with no or
some undefined ID. In addition to that having all supported DW XPCS device
IDs defined in a sinle place will improve the code maintainability and
readability.
Note while at it rename the macros to be shorter and looking alike to the
already defined NXP XPCS ID macro.
Signed-off-by: Serge Semin <[email protected]>
---
drivers/net/pcs/pcs-xpcs.c | 8 ++++----
drivers/net/pcs/pcs-xpcs.h | 3 ---
include/linux/pcs/pcs-xpcs.h | 2 ++
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 46afeb5510c0..2850122f354a 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1343,16 +1343,16 @@ static const struct xpcs_compat nxp_sja1110_xpcs_compat[DW_XPCS_INTERFACE_MAX] =
static const struct xpcs_id xpcs_id_list[] = {
{
- .id = SYNOPSYS_XPCS_ID,
- .mask = SYNOPSYS_XPCS_MASK,
+ .id = DW_XPCS_ID,
+ .mask = DW_XPCS_ID_MASK,
.compat = synopsys_xpcs_compat,
}, {
.id = NXP_SJA1105_XPCS_ID,
- .mask = SYNOPSYS_XPCS_MASK,
+ .mask = DW_XPCS_ID_MASK,
.compat = nxp_sja1105_xpcs_compat,
}, {
.id = NXP_SJA1110_XPCS_ID,
- .mask = SYNOPSYS_XPCS_MASK,
+ .mask = DW_XPCS_ID_MASK,
.compat = nxp_sja1110_xpcs_compat,
},
};
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 96c36b32ca99..369e9196f45a 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -6,9 +6,6 @@
* Author: Jose Abreu <[email protected]>
*/
-#define SYNOPSYS_XPCS_ID 0x7996ced0
-#define SYNOPSYS_XPCS_MASK 0xffffffff
-
/* Vendor regs access */
#define DW_VENDOR BIT(15)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index da3a6c30f6d2..8dfe90295f12 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -12,6 +12,8 @@
#define NXP_SJA1105_XPCS_ID 0x00000010
#define NXP_SJA1110_XPCS_ID 0x00000020
+#define DW_XPCS_ID 0x7996ced0
+#define DW_XPCS_ID_MASK 0xffffffff
/* AN mode */
#define DW_AN_C73 1
--
2.42.1
On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> Generic MDIO-device driver will support setting a custom device ID for the
> particular MDIO-device.
Why future tense? I don't see anything later in this patch set adding
this.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Dec 05, 2023 at 10:45:34AM +0000, Russell King (Oracle) wrote:
> On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> > Generic MDIO-device driver will support setting a custom device ID for the
> > particular MDIO-device.
>
> Why future tense? I don't see anything later in this patch set adding
> this.
After the next patch is applied
[PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support
the DW XPCS driver _will_ support setting custom IDs based on the
platform data and the DT compatibles. It can be used for instance to
fix the already available SJ1105 and SJ1110 MDIO bus implementations,
so instead of substituting the XPCS IDs on the PHYSID CSR reads the
driver could just pass the device ID and PMA ID via the device
platform data.
If my patch log text looks unclear anyway, just say so. I'll change it
accordingly. I guess it would be enough to say that moving is required
just to collect all the IDs in a single place.
-Serge(y)
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Dec 05, 2023 at 02:14:34PM +0300, Serge Semin wrote:
> On Tue, Dec 05, 2023 at 10:45:34AM +0000, Russell King (Oracle) wrote:
> > On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> > > Generic MDIO-device driver will support setting a custom device ID for the
> > > particular MDIO-device.
> >
> > Why future tense? I don't see anything later in this patch set adding
> > this.
>
> After the next patch is applied
> [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support
> the DW XPCS driver _will_ support setting custom IDs based on the
> platform data and the DT compatibles.
What is confusing is that the sentence makes it sound like it's some
generic driver that can be used for any PCS, whereas in reality it is
_this_ XPCS driver which is not generic.
"This driver will support setting a custom device ID in a future patch."
or explicitly state the summary line of the patch concerned so one can
refer to it. Future references are difficult to find whether they're in
email and especially once they're merged into git.
> It can be used for instance to
> fix the already available SJ1105 and SJ1110 MDIO bus implementations,
> so instead of substituting the XPCS IDs on the PHYSID CSR reads the
> driver could just pass the device ID and PMA ID via the device
> platform data.
>
> If my patch log text looks unclear anyway, just say so. I'll change it
> accordingly. I guess it would be enough to say that moving is required
> just to collect all the IDs in a single place.
You need to adjust your attitude - I did exactly that. There was
something which I didn't understand, so I raised the issue. Sorry
for spotting a problem, but do you always get arsey when a reviewer
picks up on something wrong? If that's your attitude, then for this
entire series: NAK.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> In addition to that having all supported DW XPCS device IDs defined in
> a sinle place will improve the code maintainability and readability.
single
On Tue, Dec 05, 2023 at 11:22:50AM +0000, Russell King (Oracle) wrote:
> On Tue, Dec 05, 2023 at 02:14:34PM +0300, Serge Semin wrote:
> > On Tue, Dec 05, 2023 at 10:45:34AM +0000, Russell King (Oracle) wrote:
> > > On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> > > > Generic MDIO-device driver will support setting a custom device ID for the
> > > > particular MDIO-device.
> > >
> > > Why future tense? I don't see anything later in this patch set adding
> > > this.
> >
> > After the next patch is applied
> > [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support
> > the DW XPCS driver _will_ support setting custom IDs based on the
> > platform data and the DT compatibles.
>
> What is confusing is that the sentence makes it sound like it's some
> generic driver that can be used for any PCS, whereas in reality it is
> _this_ XPCS driver which is not generic.
>
> "This driver will support setting a custom device ID in a future patch."
> or explicitly state the summary line of the patch concerned so one can
> refer to it. Future references are difficult to find whether they're in
> email and especially once they're merged into git.
Ok. I'll convert the patch log to be less confusing. As I already said
to Vladimir writing sometimes overcomplicated messages my eternal
problem.
>
> > It can be used for instance to
> > fix the already available SJ1105 and SJ1110 MDIO bus implementations,
> > so instead of substituting the XPCS IDs on the PHYSID CSR reads the
> > driver could just pass the device ID and PMA ID via the device
> > platform data.
> >
> > If my patch log text looks unclear anyway, just say so. I'll change it
> > accordingly. I guess it would be enough to say that moving is required
> > just to collect all the IDs in a single place.
>
> You need to adjust your attitude - I did exactly that. There was
> something which I didn't understand, so I raised the issue. Sorry
> for spotting a problem, but do you always get arsey when a reviewer
> picks up on something wrong? If that's your attitude, then for this
> entire series: NAK.
I'm sorry if what I wrote sounded like I was arsey. I didn't mean it
at all, really. By this sentence:
> I guess it would be enough to say that moving is required
> just to collect all the IDs in a single place.
I meant that _I_ should have just stated in the log message that
moving was required to collect all the IDs in a single place. The
rest of the text was redundant and caused confusion what you pointed
out to.
-Serge(y)
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Dec 05, 2023 at 01:27:55PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> > In addition to that having all supported DW XPCS device IDs defined in
> > a sinle place will improve the code maintainability and readability.
>
> single
Got it. Thanks. Seeing there are so many comments about the log
messages I'll double check them on v2.
-Serge(y)