Thread (6 messages) 6 messages, 3 authors, 8d ago

Re: [PATCH net-next 1/2] net: pcs: rzn1-miic: Make usage of miic_port_max consistent

flat view

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2026-09-25 16:53:21
Also in: linux-renesas-soc, lkml

Hi Kyle,

On Fri, 25 Sept 2026 at 17:20, Kyle Hendry via B4 Relay
[off-list ref] wrote:
From: Kyle Hendry <redacted>

miic_port_max is used both as the last port number and the port count
which can be different depending on SoC numbering. Use compile time
information to always set this as count and fix logic that was expecting
the last port number.

Signed-off-by: Kyle Hendry <redacted>
Thanks for your patch!
quoted hunk ↗ jump to hunk
--- a/drivers/net/pcs/pcs-rzn1-miic.c
+++ b/drivers/net/pcs/pcs-rzn1-miic.c
@@ -59,6 +59,8 @@

 #define MIIC_MAX_NUM_RSTS              2

+#define MIIC_PORT_END(x) ((x)->miic_port_start + (x)->miic_port_max - 1)
+
 /**
  * struct modctrl_match - Matching table entry for  convctrl configuration
  *                       See section 8.2.1 of manual.
@@ -222,7 +224,7 @@ enum miic_type {
  * @index_to_string: String representations of the index values
  * @index_to_string_count: Number of entries in the index_to_string array
  * @miic_port_start: MIIC port start number
- * @miic_port_max: Maximum MIIC supported
+ * @miic_port_max: Count of total MIIC ports supported
miic_port_num_total?
"max" has a different meaning.
quoted hunk ↗ jump to hunk
  * @sw_mode_mask: Switch mode mask
  * @reset_ids: Reset names array
  * @reset_count: Number of entries in the reset_ids array
@@ -482,7 +484,7 @@ struct phylink_pcs *miic_create(struct device *dev, struct device_node *np)

        miic = platform_get_drvdata(pdev);
        of_data = miic->of_data;
-       if (port > of_data->miic_port_max || port < of_data->miic_port_start) {
+       if (port > MIIC_PORT_END(of_data) || port < of_data->miic_port_start) {
IMHO the asymmetry makes the code harder to read.

As this changes the logic, I assume this is a fix?
quoted hunk ↗ jump to hunk
                put_device(&pdev->dev);
                return ERR_PTR(-EINVAL);
        }
@@ -822,7 +824,7 @@ static struct miic_of_data rzn1_miic_of_data = {
        .index_to_string = index_to_string,
        .index_to_string_count = ARRAY_SIZE(index_to_string),
        .miic_port_start = 1,
-       .miic_port_max = 5,
+       .miic_port_max = ARRAY_SIZE(index_to_string) - 1,
Why the -1? Oh, because the first entry of the array is not included.
quoted hunk ↗ jump to hunk
        .sw_mode_mask = GENMASK(4, 0),
        .init_unlock_lock_regs = true,
        .miic_write = miic_reg_writel_unlocked,
@@ -838,7 +840,7 @@ static struct miic_of_data rzt2h_miic_of_data = {
        .index_to_string = rzt2h_index_to_string,
        .index_to_string_count = ARRAY_SIZE(rzt2h_index_to_string),
        .miic_port_start = 0,
-       .miic_port_max = 4,
+       .miic_port_max = ARRAY_SIZE(rzt2h_index_to_string) - 1,
Why the -1? Oh, because the first entry of the array is not included.
And it is not related to .miic_port_start, which is zero here?
        .sw_mode_mask = GENMASK(2, 0),
        .reset_ids = rzt2h_reset_ids,
        .reset_count = ARRAY_SIZE(rzt2h_reset_ids),
I'm not sure this is an improvement at all...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help