Re: [PATCH net-next v2 2/2] r8152: Add support for the RTL8157 hardware

[Birger Koblitz]

On 19/03/2026 5:42 pm, Andrew Lunn wrote:
> > +static int wait_cmd_ready(struct r8152 *tp, u16 cmd)
> > +{
> > +	int i, ret = 0;
> > +
> > +	for (i = 0; i < 10; i++) {
> > +		u16 ocp_data = ocp_read_word(tp, MCU_TYPE_USB, cmd);
> > +
> > +		if (!(ocp_data & ADV_CMD_BUSY))
> > +			break;
> > +		usleep_range(1000, 2000);
> > +	}
> > +
> > +	if (i == 10)
> > +		ret = -ETIMEDOUT;
> > +
> > +	return ret;
>
> It is better practice to use one of the helpers from linux/iopoll.h.
Will do. I think poll_timeout_us should work nicely. Thanks for the pointer!

> > -static int r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc,
> > +static int r8152_tx_csum(struct r8152 *tp, void *d,
> >   			 struct sk_buff *skb, u32 len)
> >   {
> > +	struct rx_desc *desc = d;
> >   	u32 mss = skb_shinfo(skb)->gso_size;
> >   	u32 opts1, opts2 = 0;
> >   	int ret = TX_CSUM_SUCCESS;
>
> Reversed Christmas tree is already messed up here, but please don't
> make it worse. Add desc after mss.
Will do.

> > +static void r8157_rx_vlan_tag(void *desc, struct sk_buff *skb)
> > +{
> > +	struct rx_desc_v2 *d = desc;
> > +	u32 opts1 = le32_to_cpu(d->opts1);
>
> And here you need to move the assignment into the body in order to
> keep to reverse Christmas tree.
Will do.

Thanks for your review, Andrew!

Birger