[bug report] ata: ceva: Move sata port phy oob settings to device-tree

[bug report] ata: ceva: Move sata port phy oob settings to device-tree

[Dan Carpenter]

Hello Anurag Kumar Vulisha,

The patch fe8365bbf8ac: "ata: ceva: Move sata port phy oob settings
to device-tree" from Aug 21, 2017, leads to the following static
checker warning:

	drivers/ata/ahci_ceva.c:273 ceva_ahci_probe()
	warn: passing casted pointer '&cevapriv->pp5c[1]' to 'of_property_read_u16_array()' 32 vs 16.

drivers/ata/ahci_ceva.c
   260          if (of_property_read_u8_array(np, "ceva,p1-burst-params",
   261                                          (u8 *)&cevapriv->pp4c[1], 4) < 0) {
   262                  dev_warn(dev, "ceva,p1-burst-params property not defined\n");
   263                  return -EINVAL;
   264          }
   265  
   266          /* Read phy RETRY interval timing value from device-tree */
   267          if (of_property_read_u16_array(np, "ceva,p0-retry-params",
   268                                          (u16 *)&cevapriv->pp5c[0], 2) < 0) {
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   269                  dev_warn(dev, "ceva,p0-retry-params property not defined\n");
   270                  return -EINVAL;
   271          }
   272  
   273          if (of_property_read_u16_array(np, "ceva,p1-retry-params",
   274                                          (u16 *)&cevapriv->pp5c[1], 2) < 0) {
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^

I'm confused what's going on here.  We're reading two arrays of u16 into
an array of u32.  Why are we not just reading 2 separate u32 values?
Does this code have to work on big endian systems because I bet it
doesn't.

   275                  dev_warn(dev, "ceva,p1-retry-params property not defined\n");
   276                  return -EINVAL;
   277          }
   278  

regards,
dan carpenter

RE: [bug report] ata: ceva: Move sata port phy oob settings to device-tree

[Anurag Kumar Vulisha]

+ Michal

Hi Dan,

Please find my comments inline

>-----Original Message-----
>From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
>Sent: Tuesday, November 07, 2017 5:12 PM
>To: Anurag Kumar Vulisha <anuragku@xilinx.com>
>Cc: linux-ide@vger.kernel.org
>Subject: [bug report] ata: ceva: Move sata port phy oob settings to device-tree
>
>Hello Anurag Kumar Vulisha,
>
>The patch fe8365bbf8ac: "ata: ceva: Move sata port phy oob settings to device-
>tree" from Aug 21, 2017, leads to the following static checker warning:
>
>	drivers/ata/ahci_ceva.c:273 ceva_ahci_probe()
>	warn: passing casted pointer '&cevapriv->pp5c[1]' to
>'of_property_read_u16_array()' 32 vs 16.
>
>drivers/ata/ahci_ceva.c
>   260          if (of_property_read_u8_array(np, "ceva,p1-burst-params",
>   261                                          (u8 *)&cevapriv->pp4c[1], 4) < 0) {
>   262                  dev_warn(dev, "ceva,p1-burst-params property not defined\n");
>   263                  return -EINVAL;
>   264          }
>   265
>   266          /* Read phy RETRY interval timing value from device-tree */
>   267          if (of_property_read_u16_array(np, "ceva,p0-retry-params",
>   268                                          (u16 *)&cevapriv->pp5c[0], 2) < 0) {
>                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>   269                  dev_warn(dev, "ceva,p0-retry-params property not defined\n");
>   270                  return -EINVAL;
>   271          }
>   272
>   273          if (of_property_read_u16_array(np, "ceva,p1-retry-params",
>   274                                          (u16 *)&cevapriv->pp5c[1], 2) < 0) {
>                                                ^^^^^^^^^^^^^^^^^^^^^^^^^
>
>I'm confused what's going on here.  We're reading two arrays of u16 into an
>array of u32.  Why are we not just reading 2 separate u32 values?
>Does this code have to work on big endian systems because I bet it doesn't.
>

[Anurag Kumar Vulisha] Yes we are reading two u16 values into a u32. This is because we have two 16bit values RIT & RCT which gets genreated based on the SATA refclk being used . As we have different platforms which are using different sata  refclk and also each port can operate on different refclk,  we read the two u16 values from DT node and update it in the vendor control register. The below is the example of the RIT & RCT values that has been updated in the ahci-ceva.txt

- ceva,p0-retry-params: Retry interval timing value for port 0.               
                        The fields for the above parameter must be as shown below:
                        ceva,phy-retry-params = /bits/ 16 <RIT RCT>;            
                        RIT:  Retry Interval Timer.                             
                        RCT:  Rate Change Timer.
Example:
ceva,p0-retry-params = /bits/ 16 <0x0216 0x7F06>;

Thanks,
Anurag Kumar Vulisha

>   275                  dev_warn(dev, "ceva,p1-retry-params property not defined\n");
>   276                  return -EINVAL;
>   277          }
>   278
>
>regards,
>dan carpenter

Re: [bug report] ata: ceva: Move sata port phy oob settings to device-tree

[Dan Carpenter]

Ah, actually that makes sense then.  Thanks for the explanation.

regards,
dan carpenter