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

[Hau]

> -----Original Message-----
> From: Francois Romieu [mailto:romieu@xxxxxxxxxxxxx]
> Sent: Friday, February 2, 2018 7:27 AM
> To: Hau <hau@xxxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; nic_swsd <nic_swsd@xxxxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next] r8169: add module param for control of ASPM
> disable
> 
> Chunhao Lin <hau@xxxxxxxxxxx> :
> [...]
> > @@ -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.