[patch v2] sata, highbank: clear whole array in highbank_initialize_phys()

[patch v2] sata, highbank: clear whole array in highbank_initialize_phys()

[Dan Carpenter]

The original code used the wrong parameter to clear tx_atten[].  It
passed the number of elements instead of sizeof() the array to memset.

The other potential issue was that cphy_base[] wasn't cleared.  I'm not
sure if that was a real problem or not, but I have cleared it in my
patch.

Instead of using memset(), this patch uses empty initializers as a
cleanup.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: used {} initialization instead of memset.  I also just went ahead
    and cleared cphy_base[].

diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index 7f5e5d9..ea3b3dc 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -343,13 +343,11 @@ static int highbank_initialize_phys(struct device *dev, void __iomem *addr)
 {
 	struct device_node *sata_node = dev->of_node;
 	int phy_count = 0, phy, port = 0, i;
-	void __iomem *cphy_base[CPHY_PHY_COUNT];
-	struct device_node *phy_nodes[CPHY_PHY_COUNT];
-	u32 tx_atten[CPHY_PORT_COUNT];
+	void __iomem *cphy_base[CPHY_PHY_COUNT] = {};
+	struct device_node *phy_nodes[CPHY_PHY_COUNT] = {};
+	u32 tx_atten[CPHY_PORT_COUNT] = {};
 
 	memset(port_data, 0, sizeof(struct phy_lane_info) * CPHY_PORT_COUNT);
-	memset(phy_nodes, 0, sizeof(struct device_node*) * CPHY_PHY_COUNT);
-	memset(tx_atten, 0xff, CPHY_PORT_COUNT);
 
 	do {
 		u32 tmp;

Re: [patch v2] sata, highbank: clear whole array in highbank_initialize_phys()

[Rob Herring]

On 10/18/2013 03:44 AM, Dan Carpenter wrote:
> The original code used the wrong parameter to clear tx_atten[].  It
> passed the number of elements instead of sizeof() the array to memset.
> 
> The other potential issue was that cphy_base[] wasn't cleared.  I'm not
> sure if that was a real problem or not, but I have cleared it in my
> patch.
> 
> Instead of using memset(), this patch uses empty initializers as a
> cleanup.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Rob Herring <rob.herring@calxeda.com>

> ---
> v2: used {} initialization instead of memset.  I also just went ahead
>     and cleared cphy_base[].
> 
> diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
> index 7f5e5d9..ea3b3dc 100644
> --- a/drivers/ata/sata_highbank.c
> +++ b/drivers/ata/sata_highbank.c
> @@ -343,13 +343,11 @@ static int highbank_initialize_phys(struct device *dev, void __iomem *addr)
>  {
>  	struct device_node *sata_node = dev->of_node;
>  	int phy_count = 0, phy, port = 0, i;
> -	void __iomem *cphy_base[CPHY_PHY_COUNT];
> -	struct device_node *phy_nodes[CPHY_PHY_COUNT];
> -	u32 tx_atten[CPHY_PORT_COUNT];
> +	void __iomem *cphy_base[CPHY_PHY_COUNT] = {};
> +	struct device_node *phy_nodes[CPHY_PHY_COUNT] = {};
> +	u32 tx_atten[CPHY_PORT_COUNT] = {};
>  
>  	memset(port_data, 0, sizeof(struct phy_lane_info) * CPHY_PORT_COUNT);
> -	memset(phy_nodes, 0, sizeof(struct device_node*) * CPHY_PHY_COUNT);
> -	memset(tx_atten, 0xff, CPHY_PORT_COUNT);
>  
>  	do {
>  		u32 tmp;
> 

Re: [patch v2] sata, highbank: clear whole array in highbank_initialize_phys()

[Tejun Heo]

On Fri, Oct 18, 2013 at 11:44:09AM +0300, Dan Carpenter wrote:
> The original code used the wrong parameter to clear tx_atten[].  It
> passed the number of elements instead of sizeof() the array to memset.
> 
> The other potential issue was that cphy_base[] wasn't cleared.  I'm not
> sure if that was a real problem or not, but I have cleared it in my
> patch.
> 
> Instead of using memset(), this patch uses empty initializers as a
> cleanup.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied to libata/for-3.13.  I didn't cc stable as it doesn't look
like it caused actual problems.  Reading the code, I'm curious how
port_data[] is supposed to work.  It's a static array used throughout
the driver which is zeroed on each probe(), which doesn't make whole
lot of sense - if I connect a second sata_highbank controller in, the
struct used by the first one will be cleared.  Maybe the whole PHY
thing is supposed to be completely static, but if so, it'd be great if
the code is explicitly structured / explained that way.

Thanks.

-- 
tejun