Re: [PATCH v2 4/7] staging: mt7621-pci: fix reset lines for each pcie port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Nov 24, 2018 at 06:54:54PM +0100, Sergio Paracuellos wrote:
> Depending of chip revision reset lines are inverted. It is also
> necessary to read PCIE_FTS_NUM register before enabling the phy.
> Hence update the code to achieve this.
> 
> Fixes: 745eeeac68d7: "staging: mt7621-pci: factor out 'mt7621_pcie_enable_port'
> function"
> Reported-by: NeilBrown <neil@xxxxxxxxxx>
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> ---
>  drivers/staging/mt7621-pci/pci-mt7621.c | 38 +++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
> index ba81b34dc1b7..1b63706e129b 100644
> --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> @@ -412,6 +412,33 @@ static void mt7621_enable_phy(struct mt7621_pcie_port *port)
>  	set_phy_for_ssc(port);
>  }
>  
> +static inline void mt7621_control_assert(struct mt7621_pcie_port *port)
> +{
> +	u32 chip_rev_id = rt_sysc_r32(MT7621_CHIP_REV_ID);
> +
> +	if ((chip_rev_id & 0xFFFF) == CHIP_REV_MT7621_E2)
> +		reset_control_assert(port->pcie_rst);
> +	else
> +		reset_control_deassert(port->pcie_rst);
> +}
> +
> +static inline void mt7621_control_deassert(struct mt7621_pcie_port *port)
> +{
> +	u32 chip_rev_id = rt_sysc_r32(MT7621_CHIP_REV_ID);
> +
> +	if ((chip_rev_id & 0xFFFF) == CHIP_REV_MT7621_E2)
> +		reset_control_deassert(port->pcie_rst);
> +	else
> +		reset_control_assert(port->pcie_rst);
> +}

The commit message is very good that on some chips assert and deassert
mean the opposite but I feel like this should be commented in the code
as well or people reading this code will be very confused.

Also it would be better if we could change this from a white list to a
black list.  In other words, if they were to come out with new revs
of the hardware, we should assume that assert means assert and deassert
means deassert.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux