[PATCH 1/9] dma: pl330: rip out broken, redundant ID probing

[Grant Likely]

On Mon, 10 Jun 2013 19:34:37 +0100, Will Deacon <will.deacon at arm.com> wrote:
> The PL330 driver probes the peripheral and primecell IDs of the device to
> make sure that it is indeed an AMBA PL330. However, it does this by
> making byte accesses to a device mapping of the word-aligned ID
> registers, which is either UNPREDICTABLE or generates an alignment fault
> (depending on the presence of the virtualisation extensions).
> 
> Rather than fix this code, we can actually rip most of it out and let
> the AMBA bus driver correctly do the probing for us.
> 
> Cc: Jassi Brar <jaswinder.singh at linaro.org>
> Cc: Vinod Koul <vinod.koul at intel.com>
> Signed-off-by: Will Deacon <will.deacon at arm.com>

Right, device drivers shouldn't be cosing their own primecell ID parser.

Acked-by: Grant Likely <grant.likely at linaro.org>

> ---
>  drivers/dma/pl330.c | 27 +++------------------------
>  1 file changed, 3 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 24e0754..22e2a8f 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -157,7 +157,6 @@ enum pl330_reqtype {
>  #define PERIPH_REV_R0P0		0
>  #define PERIPH_REV_R1P0		1
>  #define PERIPH_REV_R1P1		2
> -#define PCELL_ID		0xff0
>  
>  #define CR0_PERIPH_REQ_SET	(1 << 0)
>  #define CR0_BOOT_EN_SET		(1 << 1)
> @@ -193,8 +192,6 @@ enum pl330_reqtype {
>  #define INTEG_CFG		0x0
>  #define PERIPH_ID_VAL		((PART << 0) | (DESIGNER << 12))
>  
> -#define PCELL_ID_VAL		0xb105f00d
> -
>  #define PL330_STATE_STOPPED		(1 << 0)
>  #define PL330_STATE_EXECUTING		(1 << 1)
>  #define PL330_STATE_WFE			(1 << 2)
> @@ -292,7 +289,6 @@ static unsigned cmd_line;
>  /* Populated by the PL330 core driver for DMA API driver's info */
>  struct pl330_config {
>  	u32	periph_id;
> -	u32	pcell_id;
>  #define DMAC_MODE_NS	(1 << 0)
>  	unsigned int	mode;
>  	unsigned int	data_bus_width:10; /* In number of bits */
> @@ -650,19 +646,6 @@ static inline bool _manager_ns(struct pl330_thread *thrd)
>  	return (pl330->pinfo->pcfg.mode & DMAC_MODE_NS) ? true : false;
>  }
>  
> -static inline u32 get_id(struct pl330_info *pi, u32 off)
> -{
> -	void __iomem *regs = pi->base;
> -	u32 id = 0;
> -
> -	id |= (readb(regs + off + 0x0) << 0);
> -	id |= (readb(regs + off + 0x4) << 8);
> -	id |= (readb(regs + off + 0x8) << 16);
> -	id |= (readb(regs + off + 0xc) << 24);
> -
> -	return id;
> -}
> -
>  static inline u32 get_revision(u32 periph_id)
>  {
>  	return (periph_id >> PERIPH_REV_SHIFT) & PERIPH_REV_MASK;
> @@ -1986,9 +1969,6 @@ static void read_dmac_config(struct pl330_info *pi)
>  	pi->pcfg.num_events = val;
>  
>  	pi->pcfg.irq_ns = readl(regs + CR3);
> -
> -	pi->pcfg.periph_id = get_id(pi, PERIPH_ID);
> -	pi->pcfg.pcell_id = get_id(pi, PCELL_ID);
>  }
>  
>  static inline void _reset_thread(struct pl330_thread *thrd)
> @@ -2098,10 +2078,8 @@ static int pl330_add(struct pl330_info *pi)
>  	regs = pi->base;
>  
>  	/* Check if we can handle this DMAC */
> -	if ((get_id(pi, PERIPH_ID) & 0xfffff) != PERIPH_ID_VAL
> -	   || get_id(pi, PCELL_ID) != PCELL_ID_VAL) {
> -		dev_err(pi->dev, "PERIPH_ID 0x%x, PCELL_ID 0x%x !\n",
> -			get_id(pi, PERIPH_ID), get_id(pi, PCELL_ID));
> +	if ((pi->pcfg.periph_id & 0xfffff) != PERIPH_ID_VAL) {
> +		dev_err(pi->dev, "PERIPH_ID 0x%x !\n", pi->pcfg.periph_id);
>  		return -EINVAL;
>  	}
>  
> @@ -2922,6 +2900,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>  	if (ret)
>  		return ret;
>  
> +	pi->pcfg.periph_id = adev->periphid;
>  	ret = pl330_add(pi);
>  	if (ret)
>  		goto probe_err1;
> -- 
> 1.8.2.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.