LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: davem@davemloft.net, kuba@kernel.org, robh+dt@kernel.org,
	UNGLinuxDriver@microchip.com, p.zabel@pengutronix.de,
	linux@armlinux.org.uk, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2 3/6] net: lan966x: add port module support
Date: Wed, 24 Nov 2021 18:59:34 +0100	[thread overview]
Message-ID: <YZ59hpDWjNjvx5kP@lunn.ch> (raw)
In-Reply-To: <20211123135517.4037557-4-horatiu.vultur@microchip.com>

> +static void lan966x_ifh_inject(u32 *ifh, size_t val, size_t pos, size_t length)
> +{
> +	int i;
> +
> +	for (i = pos; i < pos + length; ++i) {
> +		if (val & BIT(i - pos))
> +			ifh[IFH_LEN - i / 32 - 1] |= BIT(i % 32);
> +		else
> +			ifh[IFH_LEN - i / 32 - 1] &= ~(BIT(i % 32));
> +	}
> +}
> +
> +static void lan966x_gen_ifh(u32 *ifh, struct lan966x_frame_info *info,
> +			    struct lan966x *lan966x)
> +{
> +	lan966x_ifh_inject(ifh, 1, IFH_POS_BYPASS, 1);
> +	lan966x_ifh_inject(ifh, info->port, IFH_POS_DSTS, IFH_WID_DSTS);
> +	lan966x_ifh_inject(ifh, info->qos_class, IFH_POS_QOS_CLASS,
> +			   IFH_WID_QOS_CLASS);
> +	lan966x_ifh_inject(ifh, info->ipv, IFH_POS_IPV, IFH_WID_IPV);
> +}
> +

> +	/* Write IFH header */
> +	for (i = 0; i < IFH_LEN; ++i) {
> +		/* Wait until the fifo is ready */
> +		while (!(QS_INJ_STATUS_FIFO_RDY_GET(lan_rd(lan966x, QS_INJ_STATUS)) &
> +			 BIT(grp)))
> +			;
> +
> +		lan_wr((__force u32)cpu_to_be32(ifh[i]), lan966x,
> +		       QS_INJ_WR(grp));

There is a lot of magic going on here constructing the IFH. Is it
possible to define the structure using bit fields and __be32. You
should then be able to skip this cpu_to_be32 and the ugly cast. And
the actual structure should be a lot clearer.

> +static int lan966x_rx_frame_word(struct lan966x *lan966x, u8 grp, bool ifh,
> +				 u32 *rval)
> +{
> +	u32 bytes_valid;
> +	u32 val;
> +
> +	val = lan_rd(lan966x, QS_XTR_RD(grp));
> +	if (val == XTR_NOT_READY) {
> +		if (ifh)
> +			return -EIO;
> +
> +		do {
> +			val = lan_rd(lan966x, QS_XTR_RD(grp));
> +		} while (val == XTR_NOT_READY);

I would add some sort of timeout here, just in case the hardware
breaks. You have quite a few such loops, it would be better to make
use of the helpers in linux/iopoll.h.


  reply	other threads:[~2021-11-24 17:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23 13:55 [PATCH net-next v2 0/6] net: lan966x: Add lan966x switch driver Horatiu Vultur
2021-11-23 13:55 ` [PATCH net-next v2 1/6] dt-bindings: net: lan966x: Add lan966x-switch bindings Horatiu Vultur
2021-11-23 13:55 ` [PATCH net-next v2 2/6] net: lan966x: add the basic lan966x driver Horatiu Vultur
2021-11-23 15:31   ` Denis Kirjanov
2021-11-24  8:13     ` Horatiu Vultur
2021-11-24  3:30   ` Jakub Kicinski
2021-11-24  8:14     ` Horatiu Vultur
2021-11-23 13:55 ` [PATCH net-next v2 3/6] net: lan966x: add port module support Horatiu Vultur
2021-11-24 17:59   ` Andrew Lunn [this message]
2021-11-25  9:26     ` Horatiu Vultur
2021-11-25 14:59       ` Andrew Lunn
2021-11-26  9:12         ` Horatiu Vultur
2021-11-23 13:55 ` [PATCH net-next v2 4/6] net: lan966x: add mactable support Horatiu Vultur
2021-11-23 13:55 ` [PATCH net-next v2 5/6] net: lan966x: add ethtool configuration and statistics Horatiu Vultur
2021-11-23 13:55 ` [PATCH net-next v2 6/6] net: lan966x: Update MAINTAINERS to include lan966x driver Horatiu Vultur

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=YZ59hpDWjNjvx5kP@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=horatiu.vultur@microchip.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH net-next v2 3/6] net: lan966x: add port module support' \
    /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

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).