netdev - Re: [PATCH netnext 04/16] net: aquantia: link interrupt handling functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d63c61c1-77d6-185c-c208-0225e4337185@gmail.com>
Date:   Sat, 13 Apr 2019 15:37:00 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Igor Russkikh <Igor.Russkikh@...antia.com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Nikita Danilov <Nikita.Danilov@...antia.com>,
        Dmitry Bogdanov <Dmitry.Bogdanov@...antia.com>
Subject: Re: [PATCH netnext 04/16] net: aquantia: link interrupt handling
 functions

On 13.04.2019 14:31, Igor Russkikh wrote:
> Define link interrupt handler and work task.
> 
> Signed-off-by: Nikita Danilov <ndanilov@...antia.com>
> Signed-off-by: Igor Russkikh <igor.russkikh@...antia.com>
> ---
>  .../net/ethernet/aquantia/atlantic/aq_nic.c   | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> index 059df86e8e37..da4916195655 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -161,6 +161,28 @@ static int aq_nic_update_link_status(struct aq_nic_s *self)
>  	return 0;
>  }
>  
> +static void aq_nic_update_link_task_cb(struct work_struct *w)
> +{
> +	struct aq_nic_s *self = container_of(w, struct aq_nic_s,
> +					     link_update_task);
> +
> +	aq_nic_update_link_status(self);
> +
> +	self->aq_hw_ops->hw_irq_enable(self->aq_hw,
> +				       BIT(self->aq_nic_cfg.link_irq_vec));
> +}
> +
> +static irqreturn_t aq_linkstate_isr(int irq, void *private)
> +{
> +	struct aq_nic_s *self = private;
> +
> +	if (!self)
> +		return IRQ_NONE;
> +
> +	schedule_work(&self->link_update_task);
> +	return IRQ_HANDLED;
> +}
> +
If you just schedule a task from the hard irq handler, why not using
a threaded interrupt? And a further question because I worked on the
Aquantia PHY driver: I assume the integrated PHY's are identical or
at least very similar to the external ones like AQR107. Did you ever
consider to switch the PHY handling part of this driver to phylib?
This may help to reduce complexity and code size of the driver.

>  static void aq_nic_service_timer_cb(struct timer_list *t)
>  {
>  	struct aq_nic_s *self = from_timer(self, t, service_timer);
> @@ -297,6 +319,8 @@ int aq_nic_init(struct aq_nic_s *self)
>  		self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i])
>  		aq_vec_init(aq_vec, self->aq_hw_ops, self->aq_hw);
>  
> +	INIT_WORK(&self->link_update_task, aq_nic_update_link_task_cb);
> +
>  	netif_carrier_off(self->ndev);
>  
>  err_exit:
> @@ -873,6 +897,7 @@ int aq_nic_stop(struct aq_nic_s *self)
>  	netif_carrier_off(self->ndev);
>  
>  	del_timer_sync(&self->service_timer);
> +	cancel_work_sync(&self->link_update_task);
>  
>  	self->aq_hw_ops->hw_irq_disable(self->aq_hw, AQ_CFG_IRQ_MASK);
>  
>