Re: [PATCH net v3] net: lan743x: Don't sleep in atomic context

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Tue, Jun 27, 2023 at 03:50:00AM +0000, Moritz Fischer wrote:

Hi Moritz,

> dev_set_rx_mode() grabs a spin_lock, and the lan743x implementation
> proceeds subsequently to go to sleep using readx_poll_timeout().
> 
> Introduce a helper wrapping the readx_poll_timeout_atomic() function
> and use it to replace the calls to readx_polL_timeout().

nit: weird typo with capital L

> 
> Fixes: 23f0703c125b ("lan743x: Add main source files for new lan743x driver")
> Cc: stable@vger.kernel.org
> Cc: Bryan Whitehead <bryan.whitehead@microchip.com>
> Cc: UNGLinuxDriver@microchip.com
> Signed-off-by: Moritz Fischer <moritzf@google.com>
> ---
> 
> Changes from v2:
> - Incorporate suggestion from Jakub

suggestions were to skip the ternary operator i believe - would be good to
spell this out here

> 
> Changes from v1:
> - Added line-breaks
> - Changed subject to target net-next
> - Removed Tested-by: tag
> 
> ---
>  drivers/net/ethernet/microchip/lan743x_main.c | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index f1bded993edc..61eadc0bca8b 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -144,6 +144,18 @@ static int lan743x_csr_light_reset(struct lan743x_adapter *adapter)
>  				  !(data & HW_CFG_LRST_), 100000, 10000000);
>  }
>  
> +static int lan743x_csr_wait_for_bit_atomic(struct lan743x_adapter *adapter,

adapter is not used in readx_poll_timeout_atomic() call, right?
can be removed.

> +					   int offset, u32 bit_mask,
> +					   int target_value, int udelay_min,
> +					   int udelay_max, int count)
> +{
> +	u32 data;
> +
> +	return readx_poll_timeout_atomic(LAN743X_CSR_READ_OP, offset, data,
> +					 target_value == !!(data & bit_mask),
> +					 udelay_max, udelay_min * count);
> +}
> +
>  static int lan743x_csr_wait_for_bit(struct lan743x_adapter *adapter,
>  				    int offset, u32 bit_mask,
>  				    int target_value, int usleep_min,
> @@ -736,8 +748,8 @@ static int lan743x_dp_write(struct lan743x_adapter *adapter,
>  	u32 dp_sel;
>  	int i;
>  
> -	if (lan743x_csr_wait_for_bit(adapter, DP_SEL, DP_SEL_DPRDY_,
> -				     1, 40, 100, 100))
> +	if (lan743x_csr_wait_for_bit_atomic(adapter, DP_SEL, DP_SEL_DPRDY_,
> +					    1, 40, 100, 100))
>  		return -EIO;
>  	dp_sel = lan743x_csr_read(adapter, DP_SEL);
>  	dp_sel &= ~DP_SEL_MASK_;
> @@ -748,8 +760,9 @@ static int lan743x_dp_write(struct lan743x_adapter *adapter,
>  		lan743x_csr_write(adapter, DP_ADDR, addr + i);
>  		lan743x_csr_write(adapter, DP_DATA_0, buf[i]);
>  		lan743x_csr_write(adapter, DP_CMD, DP_CMD_WRITE_);
> -		if (lan743x_csr_wait_for_bit(adapter, DP_SEL, DP_SEL_DPRDY_,
> -					     1, 40, 100, 100))
> +		if (lan743x_csr_wait_for_bit_atomic(adapter, DP_SEL,
> +						    DP_SEL_DPRDY_,
> +						    1, 40, 100, 100))
>  			return -EIO;
>  	}
>  
> -- 
> 2.41.0.178.g377b9f9a00-goog
> 
>