netdev - RE: [PATCH net-next] r8169: add module param for control of ASPM disable

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] [day] [month] [year] [list]

Date:   Fri, 2 Feb 2018 12:52:59 +0000
From:   Hau <hau@...ltek.com>
To:     Francois Romieu <romieu@...zoreil.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        nic_swsd <nic_swsd@...ltek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next] r8169: add module param for control of ASPM disable

> -----Original Message-----
> From: Francois Romieu [mailto:romieu@...zoreil.com]
> Sent: Friday, February 2, 2018 7:27 AM
> To: Hau <hau@...ltek.com>
> Cc: netdev@...r.kernel.org; nic_swsd <nic_swsd@...ltek.com>; linux-
> kernel@...r.kernel.org
> Subject: Re: [PATCH net-next] r8169: add module param for control of ASPM
> disable
> 
> Chunhao Lin <hau@...ltek.com> :
> [...]
> > @@ -5878,6 +5881,20 @@ static void rtl_pcie_state_l2l3_enable(struct
> rtl8169_private *tp, bool enable)
> >  	RTL_W8(Config3, data);
> >  }
> >
> > +static void rtl_hw_internal_aspm_clkreq_enable(struct rtl8169_private
> *tp,
> > +				bool enable)
> > +{
> > +	void __iomem *ioaddr = tp->mmio_addr;
> > +
> > +	if (enable) {
> > +		RTL_W8(Config2, RTL_R8(Config2) | ClkReqEn);
> > +		RTL_W8(Config5, RTL_R8(Config5) | ASPM_en);
> > +	} else {
> > +		RTL_W8(Config2, RTL_R8(Config2) & ~ClkReqEn);
> > +		RTL_W8(Config5, RTL_R8(Config5) & ~ASPM_en);
> > +	}
> > +}
> 
> s/enable(..., false)/disable()/
> 
> static void rtl_hw_internal_aspm_clkreq_enable(truct rtl8169_private *tp) {
> 	void __iomem *ioaddr = tp->mmio_addr;
> 
> 	RTL_W8(Config2, RTL_R8(Config2) | ClkReqEn);
> 	RTL_W8(Config5, RTL_R8(Config5) | ASPM_en); }
> 
> static void rtl_hw_internal_aspm_clkreq_disable(truct rtl8169_private *tp) {
> 	void __iomem *ioaddr = tp->mmio_addr;
> 
> 	RTL_W8(Config2, RTL_R8(Config2) & ~ClkReqEn);
> 	RTL_W8(Config5, RTL_R8(Config5) & ~ASPM_en); }
> 
> If you really want to factor something out, you may use helpers that set or
> clear bits according to the 3-uple (tp, register, bits) but foo_enable(..., false)
> is pointlessly convoluted.
> 
> --
> Ueimor
> 
> ------Please consider the environment before printing this e-mail.

This two bits are related to hardware ASPM function, so I put them together into one function.
I could use helper to set these bits, if doing this can make the patch more readable for kernel team.

Thanks for inform me that net-next is closed. I will resubmit this patch when net-next open.

Thanks.

------Please consider the environment before printing this e-mail.