From: "Alvin Šipraga" <ALSI@bang-olufsen.dk>
To: "DENG Qingfang" <dqfext@gmail.com>, "Alvin Šipraga" <alvin@pqrs.dk>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Heiner Kallweit <hkallweit1@gmail.com>,
Russell King <linux@armlinux.org.uk>,
Michael Rasmussen <MIR@bang-olufsen.dk>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 5/6] net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC
Date: Wed, 13 Oct 2021 10:05:21 +0000 [thread overview]
Message-ID: <32ffccb4-ff8b-5e60-ad29-e32bcb22969f@bang-olufsen.dk> (raw)
In-Reply-To: <20211013095505.55966-1-dqfext@gmail.com>
On 10/13/21 11:55 AM, DENG Qingfang wrote:
> On Tue, Oct 12, 2021 at 02:35:54PM +0200, Alvin Šipraga wrote:
>> +/* Port mapping macros
>> + *
>> + * PORT_NUM_x2y: map a port number from domain x to domain y
>> + * PORT_MASK_x2y: map a port mask from domain x to domain y
>> + *
>> + * L = logical port domain, i.e. dsa_port.index
>> + * P = physical port domain, used by the Realtek ASIC for port indexing;
>> + * for ports with internal PHYs, this is also the PHY index
>> + * E = extension port domain, used by the Realtek ASIC for managing EXT ports
>> + *
>> + * The terminology is borrowed from the vendor driver. The extension port domain
>> + * is mostly used to navigate the labyrinthine layout of EXT port configuration
>> + * registers and is not considered intuitive by the author.
>> + *
>> + * Unless a function is accessing chip registers, it should be using the logical
>> + * port domain. Moreover, function arguments for port numbers and port masks
>> + * must always be in the logical domain. The conversion must be done as close as
>> + * possible to the register access to avoid chaos.
>> + *
>> + * The mappings vary between chips in the family supported by this driver. Here
>> + * is an example of the mapping for the RTL8365MB-VC:
>> + *
>> + * L | P | E | remark
>> + * ---+---+---+--------
>> + * 0 | 0 | | user port
>> + * 1 | 1 | | user port
>> + * 2 | 2 | | user port
>> + * 3 | 3 | | user port
>> + * 4 | 6 | 1 | extension (CPU) port
>> + *
>> + * NOTE: Currently this is hardcoded for the RTL8365MB-VC. This will probably
>> + * require a rework when adding support for other chips.
>> + */
>> +#define CPU_PORT_LOGICAL_NUM 4
>> +#define CPU_PORT_LOGICAL_MASK BIT(CPU_PORT_LOGICAL_NUM)
>> +#define CPU_PORT_PHYSICAL_NUM 6
>> +#define CPU_PORT_PHYSICAL_MASK BIT(CPU_PORT_PHYSICAL_NUM)
>> +#define CPU_PORT_EXTENSION_NUM 1
>> +
>> +static u32 rtl8365mb_port_num_l2p(u32 port)
>> +{
>> + return port == CPU_PORT_LOGICAL_NUM ? CPU_PORT_PHYSICAL_NUM : port;
>> +}
>> +
>> +static u32 rtl8365mb_port_mask_l2p(u32 mask)
>> +{
>> + u32 phys_mask = mask & ~CPU_PORT_LOGICAL_MASK;
>> +
>> + if (mask & CPU_PORT_LOGICAL_MASK)
>> + phys_mask |= CPU_PORT_PHYSICAL_MASK;
>> +
>> + return phys_mask;
>> +}
>> +
>> +static u32 rtl8365mb_port_mask_p2l(u32 phys_mask)
>> +{
>> + u32 mask = phys_mask & ~CPU_PORT_PHYSICAL_MASK;
>> +
>> + if (phys_mask & CPU_PORT_PHYSICAL_MASK)
>> + mask |= CPU_PORT_LOGICAL_MASK;
>> +
>> + return mask;
>> +}
>> +
>> +#define PORT_NUM_L2P(_p) (rtl8365mb_port_num_l2p(_p))
>> +#define PORT_NUM_L2E(_p) (CPU_PORT_EXTENSION_NUM)
>> +#define PORT_MASK_L2P(_m) (rtl8365mb_port_mask_l2p(_m))
>> +#define PORT_MASK_P2L(_m) (rtl8365mb_port_mask_p2l(_m))
>
> The whole port mapping thing can be avoided if you just use port 6 as the CPU
> port.
Andrew also suggested this, but the discontinuity in port IDs seems to
be an invitation for trouble. Here is an example of a series of
functions from dsa.h:
static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
{
struct dsa_switch_tree *dst = ds->dst;
struct dsa_port *dp;
list_for_each_entry(dp, &dst->ports, list)
if (dp->ds == ds && dp->index == p)
return dp;
return NULL;
}
static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
{
return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_USER;
}
static inline u32 dsa_user_ports(struct dsa_switch *ds)
{
u32 mask = 0;
int p;
for (p = 0; p < ds->num_ports; p++)
if (dsa_is_user_port(ds, p))
mask |= BIT(p);
return mask;
}
My reading of dsa_user_ports() is that the port IDs run from 0 to
(ds->num_ports - 1). If num_ports is 5 (4 user ports and 1 CPU port, as
in my case), but the CPU is port 6, will we not dereference NULL when
calling dsa_is_user_port(ds, 4)?
>
>> +
>> +/* Chip-specific data and limits */
next prev parent reply other threads:[~2021-10-13 10:05 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-12 12:35 [PATCH net-next 0/6] net: dsa: add support for RTL8365MB-VC Alvin Šipraga
2021-10-12 12:35 ` [PATCH net-next 1/6] ether: add EtherType for proprietary Realtek protocols Alvin Šipraga
2021-10-12 13:09 ` Vladimir Oltean
2021-10-12 12:35 ` [PATCH net-next 2/6] net: dsa: move NET_DSA_TAG_RTL4_A to right place in Kconfig/Makefile Alvin Šipraga
2021-10-12 12:42 ` Vladimir Oltean
2021-10-13 10:50 ` Linus Walleij
2021-10-12 12:35 ` [PATCH net-next 3/6] dt-bindings: net: dsa: realtek-smi: document new compatible rtl8365mb Alvin Šipraga
2021-10-12 12:35 ` [PATCH net-next 4/6] net: dsa: tag_rtl8_4: add realtek 8 byte protocol 4 tag Alvin Šipraga
2021-10-12 12:50 ` Vladimir Oltean
2021-10-12 12:56 ` Alvin Šipraga
2021-10-13 9:45 ` DENG Qingfang
2021-10-13 9:52 ` Alvin Šipraga
2021-10-13 11:02 ` Linus Walleij
2021-10-12 12:35 ` [PATCH net-next 5/6] net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC Alvin Šipraga
2021-10-12 13:04 ` Vladimir Oltean
2021-10-12 13:22 ` Alvin Šipraga
2021-10-12 13:50 ` Alvin Šipraga
2021-10-12 14:03 ` Vladimir Oltean
2021-10-12 14:30 ` Alvin Šipraga
2021-10-12 15:27 ` Jakub Kicinski
2021-10-13 8:33 ` Alvin Šipraga
2021-10-13 15:13 ` Jakub Kicinski
2021-10-14 12:44 ` Alvin Šipraga
2021-10-14 14:08 ` Jakub Kicinski
2021-10-13 9:55 ` DENG Qingfang
2021-10-13 10:05 ` Alvin Šipraga [this message]
2021-10-13 10:10 ` Vladimir Oltean
2021-10-13 10:13 ` DENG Qingfang
2021-10-13 15:12 ` Linus Walleij
2021-10-14 12:11 ` Alvin Šipraga
2021-10-12 12:35 ` [PATCH net-next 6/6] net: phy: realtek: add support for RTL8365MB-VC internal PHYs Alvin Šipraga
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=32ffccb4-ff8b-5e60-ad29-e32bcb22969f@bang-olufsen.dk \
--to=alsi@bang-olufsen.dk \
--cc=MIR@bang-olufsen.dk \
--cc=alvin@pqrs.dk \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=dqfext@gmail.com \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=robh+dt@kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).