Re: [PATCH 7/9][RESEND] mvsas: get phy info.

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2008-03-27 at 10:04 -0700, Grant Grundler wrote:
> On Wed, Mar 26, 2008 at 11:55 PM, Ke Wei <kewei@marvell.com> wrote:
> > removed unused code and attached SATA address makes use of port id.
> >  enable HBA interrupt after calling sas_register_ha();
> 
> just one small nit and one question.
> 
> ...
> >  @@ -2837,20 +2812,34 @@ static void mvs_update_phyinfo(struct mvs_info *mvi, int i,
> >                         } else {
> >                                 dev_printk(KERN_DEBUG, &pdev->dev,
> >                                         "No sig fis\n");
> >  +                               phy->phy_type &= ~(PORT_TYPE_SATA);
> 
> shouldn't need '()'s around the constant.
> 
> >  +                               goto out_done;
> >                         }
> >                 }
> >  +               tmp64 = cpu_to_be64(phy->att_dev_sas_addr);
> >  +               memcpy(sas_phy->attached_sas_addr, &tmp64, SAS_ADDR_SIZE);
> 
> Why does this need to be a memcpy and not just a simple store operation?
> Is it misaligned? (even if it is, the kernel will deal with it. It
> just won't be fast.)

Because if you look in include/scsi/libsas.h, you find the definition:

struct asd_sas_phy {
[...]
	u8   attached_sas_addr[SAS_ADDR_SIZE]; /* class:RO, driver: R/W */


We're annoyingly schizophrenic on this ... most of the time we have a
sas_addr defined as u64, but not in a lot of the libsas code.

James