RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-30 Thread bharat.bhus...@freescale.com


> -Original Message-
> From: Wood Scott-B07421
> Sent: Tuesday, July 01, 2014 2:30 AM
> To: Bhushan Bharat-R65777
> Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
> reporting driver
> 
> On Sun, 2014-06-29 at 23:58 -0500, Bhushan Bharat-R65777 wrote:
> >
> > > -Original Message-
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, June 04, 2014 10:38 PM
> > > To: Bhushan Bharat-R65777
> > > Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> > > ker...@vger.kernel.org
> > > Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
> > > Fabric error reporting driver
> > >
> > > On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
> > > >
> > > > > -Original Message-
> > > > > From: Wood Scott-B07421
> > > > > Sent: Wednesday, June 04, 2014 10:12 PM
> > > > > To: Bhushan Bharat-R65777
> > > > > Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> > > > > ker...@vger.kernel.org
> > > > > Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
> > > > > Fabric error reporting driver
> > > > >
> > > > > On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
> > > > > > > +static int ccf_remove(struct platform_device *pdev) {
> > > > > > > + struct ccf_private *ccf = dev_get_drvdata(>dev);
> > > > > > > +
> > > > > > > + switch (ccf->info->version) {
> > > > > > > + case CCF1:
> > > > > > > + iowrite32be(0, >err_regs->errdis);
> > > > > > > + break;
> > > > > > > +
> > > > > > > + case CCF2:
> > > > > > > + iowrite32be(0, >err_regs->errinten);
> > > > > >
> > > > > > Do you think it is same to disable detection bits in
> > > > > > ccf->err_regs-
> > > >errdis?
> > > > >
> > > > > Disabling the interrupt is what we're aiming for here, but ccf1
> > > > > doesn't provide a way to do that separate from disabling detection.
> > > >
> > > > What I wanted to say that do we also need to disable detection
> > > > (set ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing
> > > > errinten on
> > > > ccf2 ?
> > >
> > > I don't think we "need" to.  You could argue that we should for
> > > consistency, though I think there's value in errors continuing to be
> > > detected even without the driver (e.g. can dump the registers in a
> debugger).
> >
> > Yes this comment was for consistency. Also IIUC, the state which is left 
> > when
> the driver is removed is not default reset behavior.
> 
> How many drivers leave the hardware in pristine reset state when exiting?

I do not know :)

>  And
> you could argue that having detection off by default is poor hardware design
> (enabling interrupts is another matter of course).

Ok, then can you please add a comment in _remove() function describing why 
detection is still enabled.

Thanks
-Bharat

> 
> > If we want errors to be detected then should not we have a sysfs interface?
> 
> That may be useful but it's beyond the scope of what I'm doing with this 
> patch.
> We currently don't log machine checks anywhere but via printk either.
> 
> BTW, I thought I had sent v2 of this, but I don't see it anywhere...
> I'll respin soon.
> 
> -Scott
> 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-30 Thread Scott Wood
On Sun, 2014-06-29 at 23:58 -0500, Bhushan Bharat-R65777 wrote:
> 
> > -Original Message-
> > From: Wood Scott-B07421
> > Sent: Wednesday, June 04, 2014 10:38 PM
> > To: Bhushan Bharat-R65777
> > Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> > ker...@vger.kernel.org
> > Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
> > reporting driver
> > 
> > On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
> > >
> > > > -Original Message-
> > > > From: Wood Scott-B07421
> > > > Sent: Wednesday, June 04, 2014 10:12 PM
> > > > To: Bhushan Bharat-R65777
> > > > Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> > > > ker...@vger.kernel.org
> > > > Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
> > > > Fabric error reporting driver
> > > >
> > > > On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
> > > > > > +static int ccf_remove(struct platform_device *pdev) {
> > > > > > +   struct ccf_private *ccf = dev_get_drvdata(>dev);
> > > > > > +
> > > > > > +   switch (ccf->info->version) {
> > > > > > +   case CCF1:
> > > > > > +   iowrite32be(0, >err_regs->errdis);
> > > > > > +   break;
> > > > > > +
> > > > > > +   case CCF2:
> > > > > > +   iowrite32be(0, >err_regs->errinten);
> > > > >
> > > > > Do you think it is same to disable detection bits in ccf->err_regs-
> > >errdis?
> > > >
> > > > Disabling the interrupt is what we're aiming for here, but ccf1
> > > > doesn't provide a way to do that separate from disabling detection.
> > >
> > > What I wanted to say that do we also need to disable detection (set
> > > ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing errinten on
> > > ccf2 ?
> > 
> > I don't think we "need" to.  You could argue that we should for consistency,
> > though I think there's value in errors continuing to be detected even 
> > without
> > the driver (e.g. can dump the registers in a debugger).
> 
> Yes this comment was for consistency. Also IIUC, the state which is left when 
> the driver is removed is not default reset behavior.

How many drivers leave the hardware in pristine reset state when
exiting?  And you could argue that having detection off by default is
poor hardware design (enabling interrupts is another matter of course).

> If we want errors to be detected then should not we have a sysfs interface?

That may be useful but it's beyond the scope of what I'm doing with this
patch.  We currently don't log machine checks anywhere but via printk
either.

BTW, I thought I had sent v2 of this, but I don't see it anywhere...
I'll respin soon.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-30 Thread Scott Wood
On Sun, 2014-06-29 at 23:58 -0500, Bhushan Bharat-R65777 wrote:
 
  -Original Message-
  From: Wood Scott-B07421
  Sent: Wednesday, June 04, 2014 10:38 PM
  To: Bhushan Bharat-R65777
  Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
  ker...@vger.kernel.org
  Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
  reporting driver
  
  On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
  
-Original Message-
From: Wood Scott-B07421
Sent: Wednesday, June 04, 2014 10:12 PM
To: Bhushan Bharat-R65777
Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
ker...@vger.kernel.org
Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
Fabric error reporting driver
   
On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
  +static int ccf_remove(struct platform_device *pdev) {
  +   struct ccf_private *ccf = dev_get_drvdata(pdev-dev);
  +
  +   switch (ccf-info-version) {
  +   case CCF1:
  +   iowrite32be(0, ccf-err_regs-errdis);
  +   break;
  +
  +   case CCF2:
  +   iowrite32be(0, ccf-err_regs-errinten);

 Do you think it is same to disable detection bits in ccf-err_regs-
  errdis?
   
Disabling the interrupt is what we're aiming for here, but ccf1
doesn't provide a way to do that separate from disabling detection.
  
   What I wanted to say that do we also need to disable detection (set
   ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing errinten on
   ccf2 ?
  
  I don't think we need to.  You could argue that we should for consistency,
  though I think there's value in errors continuing to be detected even 
  without
  the driver (e.g. can dump the registers in a debugger).
 
 Yes this comment was for consistency. Also IIUC, the state which is left when 
 the driver is removed is not default reset behavior.

How many drivers leave the hardware in pristine reset state when
exiting?  And you could argue that having detection off by default is
poor hardware design (enabling interrupts is another matter of course).

 If we want errors to be detected then should not we have a sysfs interface?

That may be useful but it's beyond the scope of what I'm doing with this
patch.  We currently don't log machine checks anywhere but via printk
either.

BTW, I thought I had sent v2 of this, but I don't see it anywhere...
I'll respin soon.

-Scott


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-30 Thread bharat.bhus...@freescale.com


 -Original Message-
 From: Wood Scott-B07421
 Sent: Tuesday, July 01, 2014 2:30 AM
 To: Bhushan Bharat-R65777
 Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
 ker...@vger.kernel.org
 Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
 reporting driver
 
 On Sun, 2014-06-29 at 23:58 -0500, Bhushan Bharat-R65777 wrote:
 
   -Original Message-
   From: Wood Scott-B07421
   Sent: Wednesday, June 04, 2014 10:38 PM
   To: Bhushan Bharat-R65777
   Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
   ker...@vger.kernel.org
   Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
   Fabric error reporting driver
  
   On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
   
 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, June 04, 2014 10:12 PM
 To: Bhushan Bharat-R65777
 Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
 ker...@vger.kernel.org
 Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
 Fabric error reporting driver

 On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
   +static int ccf_remove(struct platform_device *pdev) {
   + struct ccf_private *ccf = dev_get_drvdata(pdev-dev);
   +
   + switch (ccf-info-version) {
   + case CCF1:
   + iowrite32be(0, ccf-err_regs-errdis);
   + break;
   +
   + case CCF2:
   + iowrite32be(0, ccf-err_regs-errinten);
 
  Do you think it is same to disable detection bits in
  ccf-err_regs-
   errdis?

 Disabling the interrupt is what we're aiming for here, but ccf1
 doesn't provide a way to do that separate from disabling detection.
   
What I wanted to say that do we also need to disable detection
(set ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing
errinten on
ccf2 ?
  
   I don't think we need to.  You could argue that we should for
   consistency, though I think there's value in errors continuing to be
   detected even without the driver (e.g. can dump the registers in a
 debugger).
 
  Yes this comment was for consistency. Also IIUC, the state which is left 
  when
 the driver is removed is not default reset behavior.
 
 How many drivers leave the hardware in pristine reset state when exiting?

I do not know :)

  And
 you could argue that having detection off by default is poor hardware design
 (enabling interrupts is another matter of course).

Ok, then can you please add a comment in _remove() function describing why 
detection is still enabled.

Thanks
-Bharat

 
  If we want errors to be detected then should not we have a sysfs interface?
 
 That may be useful but it's beyond the scope of what I'm doing with this 
 patch.
 We currently don't log machine checks anywhere but via printk either.
 
 BTW, I thought I had sent v2 of this, but I don't see it anywhere...
 I'll respin soon.
 
 -Scott
 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-29 Thread bharat.bhus...@freescale.com


> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, June 04, 2014 10:38 PM
> To: Bhushan Bharat-R65777
> Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
> reporting driver
> 
> On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
> >
> > > -Original Message-
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, June 04, 2014 10:12 PM
> > > To: Bhushan Bharat-R65777
> > > Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> > > ker...@vger.kernel.org
> > > Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
> > > Fabric error reporting driver
> > >
> > > On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
> > > > > +static int ccf_remove(struct platform_device *pdev) {
> > > > > + struct ccf_private *ccf = dev_get_drvdata(>dev);
> > > > > +
> > > > > + switch (ccf->info->version) {
> > > > > + case CCF1:
> > > > > + iowrite32be(0, >err_regs->errdis);
> > > > > + break;
> > > > > +
> > > > > + case CCF2:
> > > > > + iowrite32be(0, >err_regs->errinten);
> > > >
> > > > Do you think it is same to disable detection bits in ccf->err_regs-
> >errdis?
> > >
> > > Disabling the interrupt is what we're aiming for here, but ccf1
> > > doesn't provide a way to do that separate from disabling detection.
> >
> > What I wanted to say that do we also need to disable detection (set
> > ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing errinten on
> > ccf2 ?
> 
> I don't think we "need" to.  You could argue that we should for consistency,
> though I think there's value in errors continuing to be detected even without
> the driver (e.g. can dump the registers in a debugger).

Yes this comment was for consistency. Also IIUC, the state which is left when 
the driver is removed is not default reset behavior.
If we want errors to be detected then should not we have a sysfs interface?

Thanks
-Bharat

> 
> -Scott
> 



RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-29 Thread bharat.bhus...@freescale.com


 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, June 04, 2014 10:38 PM
 To: Bhushan Bharat-R65777
 Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
 ker...@vger.kernel.org
 Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
 reporting driver
 
 On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
 
   -Original Message-
   From: Wood Scott-B07421
   Sent: Wednesday, June 04, 2014 10:12 PM
   To: Bhushan Bharat-R65777
   Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
   ker...@vger.kernel.org
   Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency
   Fabric error reporting driver
  
   On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
 +static int ccf_remove(struct platform_device *pdev) {
 + struct ccf_private *ccf = dev_get_drvdata(pdev-dev);
 +
 + switch (ccf-info-version) {
 + case CCF1:
 + iowrite32be(0, ccf-err_regs-errdis);
 + break;
 +
 + case CCF2:
 + iowrite32be(0, ccf-err_regs-errinten);
   
Do you think it is same to disable detection bits in ccf-err_regs-
 errdis?
  
   Disabling the interrupt is what we're aiming for here, but ccf1
   doesn't provide a way to do that separate from disabling detection.
 
  What I wanted to say that do we also need to disable detection (set
  ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing errinten on
  ccf2 ?
 
 I don't think we need to.  You could argue that we should for consistency,
 though I think there's value in errors continuing to be detected even without
 the driver (e.g. can dump the registers in a debugger).

Yes this comment was for consistency. Also IIUC, the state which is left when 
the driver is removed is not default reset behavior.
If we want errors to be detected then should not we have a sysfs interface?

Thanks
-Bharat

 
 -Scott
 



Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread Scott Wood
On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
> 
> > -Original Message-
> > From: Wood Scott-B07421
> > Sent: Wednesday, June 04, 2014 10:12 PM
> > To: Bhushan Bharat-R65777
> > Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> > ker...@vger.kernel.org
> > Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
> > reporting driver
> > 
> > On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
> > > > +static int ccf_remove(struct platform_device *pdev) {
> > > > +   struct ccf_private *ccf = dev_get_drvdata(>dev);
> > > > +
> > > > +   switch (ccf->info->version) {
> > > > +   case CCF1:
> > > > +   iowrite32be(0, >err_regs->errdis);
> > > > +   break;
> > > > +
> > > > +   case CCF2:
> > > > +   iowrite32be(0, >err_regs->errinten);
> > >
> > > Do you think it is same to disable detection bits in 
> > > ccf->err_regs->errdis?
> > 
> > Disabling the interrupt is what we're aiming for here, but ccf1 doesn't 
> > provide
> > a way to do that separate from disabling detection.
> 
> What I wanted to say that do we also need to disable detection (set
> ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing errinten on
> ccf2 ?

I don't think we "need" to.  You could argue that we should for
consistency, though I think there's value in errors continuing to be
detected even without the driver (e.g. can dump the registers in a
debugger).

-Scott


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread bharat.bhus...@freescale.com


> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, June 04, 2014 10:12 PM
> To: Bhushan Bharat-R65777
> Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
> reporting driver
> 
> On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
> > > +struct ccf_err_regs {
> > > + u32 errdet; /* 0x00 Error Detect Register */
> > > + /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
> > > + u32 errdis;
> > > + /* 0x08 Error Interrupt Enable Register (ccf2 only) */
> > > + u32 errinten;
> > > + u32 cecar;  /* 0x0c Error Capture Attribute Register */
> > > + u32 cecadrh;/* 0x10 Error Capture Address High */
> >
> > s/cecadrh/cecaddrh/g
> > This way we will be consistent with Reference manual.
> 
> It's "cecadrh" in ccf1 and "cecaddrh" in ccf2.  I suppose I should use the
> latter since "errdet/errdis/errinten" are the ccf2 names.
> 
> > > + u32 cecadrl;/* 0x14 Error Capture Address Low */
> >
> > s/cecadrl/cecaddrl/g
> >
> > > + u32 cecar2; /* 0x18 Error Capture Attribute Register 2 */
> > > +};
> > > +
> > > +/* LAE/CV also valid for errdis and errinten */
> > > +#define ERRDET_LAE   (1 << 0)  /* Local Access Error */
> > > +#define ERRDET_CV(1 << 1)  /* Coherency Violation */
> > > +#define ERRDET_CTYPE_SHIFT   26/* Capture Type (ccf2 only) */
> > > +#define ERRDET_CTYPE_MASK(0x3f << ERRDET_CTYPE_SHIFT)
> >
> > Should not this be (0x1f << ERRDET_CTYPE_SHIFT)
> 
> Yes, thanks for catching that.
> 
> > > +#define ERRDET_CAP   (1 << 31) /* Capture Valid (ccf2 only) 
> > > */
> > > +
> > > +#define CECAR_VAL(1 << 0)  /* Valid (ccf1 only) */
> > > +#define CECAR_UVT(1 << 15) /* Unavailable target ID 
> > > (ccf1) */
> > > +#define CECAR_SRCID_SHIFT_CCF1   24
> > > +#define CECAR_SRCID_MASK_CCF1(0xff << CECAR_SRCID_SHIFT_CCF1)
> > > +#define CECAR_SRCID_SHIFT_CCF2   18
> > > +#define CECAR_SRCID_MASK_CCF2(0xff << CECAR_SRCID_SHIFT_CCF2)
> > > +
> > > +#define CECADRH_ADDRH0xf
> >
> > On ccf2 this id 0xff.
> 
> OK.  I think we can get away with using 0xff on both.
> 
> > > +static int ccf_remove(struct platform_device *pdev) {
> > > + struct ccf_private *ccf = dev_get_drvdata(>dev);
> > > +
> > > + switch (ccf->info->version) {
> > > + case CCF1:
> > > + iowrite32be(0, >err_regs->errdis);
> > > + break;
> > > +
> > > + case CCF2:
> > > + iowrite32be(0, >err_regs->errinten);
> >
> > Do you think it is same to disable detection bits in ccf->err_regs->errdis?
> 
> Disabling the interrupt is what we're aiming for here, but ccf1 doesn't 
> provide
> a way to do that separate from disabling detection.

What I wanted to say that do we also need to disable detection (set ERRDET_LAE 
| ERRDET_CV bits in errdis) apart from clearing errinten on ccf2 ?

Thanks
-Bharat

> 
> -Scott
> 



Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread Scott Wood
On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
> > +struct ccf_err_regs {
> > +   u32 errdet; /* 0x00 Error Detect Register */
> > +   /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
> > +   u32 errdis;
> > +   /* 0x08 Error Interrupt Enable Register (ccf2 only) */
> > +   u32 errinten;
> > +   u32 cecar;  /* 0x0c Error Capture Attribute Register */
> > +   u32 cecadrh;/* 0x10 Error Capture Address High */
> 
> s/cecadrh/cecaddrh/g
> This way we will be consistent with Reference manual.

It's "cecadrh" in ccf1 and "cecaddrh" in ccf2.  I suppose I should use
the latter since "errdet/errdis/errinten" are the ccf2 names.

> > +   u32 cecadrl;/* 0x14 Error Capture Address Low */
> 
> s/cecadrl/cecaddrl/g
> 
> > +   u32 cecar2; /* 0x18 Error Capture Attribute Register 2 */
> > +};
> > +
> > +/* LAE/CV also valid for errdis and errinten */
> > +#define ERRDET_LAE (1 << 0)  /* Local Access Error */
> > +#define ERRDET_CV  (1 << 1)  /* Coherency Violation */
> > +#define ERRDET_CTYPE_SHIFT 26/* Capture Type (ccf2 only) */
> > +#define ERRDET_CTYPE_MASK  (0x3f << ERRDET_CTYPE_SHIFT)
> 
> Should not this be (0x1f << ERRDET_CTYPE_SHIFT)

Yes, thanks for catching that.

> > +#define ERRDET_CAP (1 << 31) /* Capture Valid (ccf2 only) */
> > +
> > +#define CECAR_VAL  (1 << 0)  /* Valid (ccf1 only) */
> > +#define CECAR_UVT  (1 << 15) /* Unavailable target ID (ccf1) */
> > +#define CECAR_SRCID_SHIFT_CCF1 24
> > +#define CECAR_SRCID_MASK_CCF1  (0xff << CECAR_SRCID_SHIFT_CCF1)
> > +#define CECAR_SRCID_SHIFT_CCF2 18
> > +#define CECAR_SRCID_MASK_CCF2  (0xff << CECAR_SRCID_SHIFT_CCF2)
> > +
> > +#define CECADRH_ADDRH  0xf
> 
> On ccf2 this id 0xff.

OK.  I think we can get away with using 0xff on both.

> > +static int ccf_remove(struct platform_device *pdev) {
> > +   struct ccf_private *ccf = dev_get_drvdata(>dev);
> > +
> > +   switch (ccf->info->version) {
> > +   case CCF1:
> > +   iowrite32be(0, >err_regs->errdis);
> > +   break;
> > +
> > +   case CCF2:
> > +   iowrite32be(0, >err_regs->errinten);
> 
> Do you think it is same to disable detection bits in ccf->err_regs->errdis?

Disabling the interrupt is what we're aiming for here, but ccf1 doesn't
provide a way to do that separate from disabling detection.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread bharat.bhus...@freescale.com


> -Original Message-
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of Scott Wood
> Sent: Saturday, May 31, 2014 3:58 AM
> To: Greg Kroah-Hartman
> Cc: linuxppc-...@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
> reporting driver
> 
> The CoreNet Coherency Fabric is part of the memory subsystem on some Freescale
> QorIQ chips.  It can report coherency violations (e.g.
> due to misusing memory that is mapped noncoherent) as well as transactions 
> that
> do not hit any local access window, or which hit a local access window with an
> invalid target ID.
> 
> Signed-off-by: Scott Wood 
> ---
> Resending to the proper list addresses -- sorry for the duplicate.
> 
>  arch/powerpc/configs/corenet32_smp_defconfig |   1 +
>  arch/powerpc/configs/corenet64_smp_defconfig |   1 +
>  drivers/memory/Kconfig   |  10 ++
>  drivers/memory/Makefile  |   1 +
>  drivers/memory/fsl-corenet-cf.c  | 246 
> +++
>  5 files changed, 259 insertions(+)
>  create mode 100644 drivers/memory/fsl-corenet-cf.c
> 
> diff --git a/arch/powerpc/configs/corenet32_smp_defconfig
> b/arch/powerpc/configs/corenet32_smp_defconfig
> index c19ff05..0c99d7e 100644
> --- a/arch/powerpc/configs/corenet32_smp_defconfig
> +++ b/arch/powerpc/configs/corenet32_smp_defconfig
> @@ -179,3 +179,4 @@ CONFIG_CRYPTO_SHA512=y  CONFIG_CRYPTO_AES=y  #
> CONFIG_CRYPTO_ANSI_CPRNG is not set  CONFIG_CRYPTO_DEV_FSL_CAAM=y
> +CONFIG_FSL_CORENET_CF=y
> diff --git a/arch/powerpc/configs/corenet64_smp_defconfig
> b/arch/powerpc/configs/corenet64_smp_defconfig
> index 5c7fa19..8fb616d 100644
> --- a/arch/powerpc/configs/corenet64_smp_defconfig
> +++ b/arch/powerpc/configs/corenet64_smp_defconfig
> @@ -175,3 +175,4 @@ CONFIG_CRYPTO_SHA256=y  CONFIG_CRYPTO_SHA512=y  #
> CONFIG_CRYPTO_ANSI_CPRNG is not set  CONFIG_CRYPTO_DEV_FSL_CAAM=y
> +CONFIG_FSL_CORENET_CF=y
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig index
> c59e9c9..fab81a1 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -61,6 +61,16 @@ config TEGRA30_MC
> analysis, especially for IOMMU/SMMU(System Memory Management
> Unit) module.
> 
> +config FSL_CORENET_CF
> + tristate "Freescale CoreNet Error Reporting"
> + depends on FSL_SOC_BOOKE
> + help
> +   Say Y for reporting of errors from the Freescale CoreNet
> +   Coherency Fabric.  Errors reported include accesses to
> +   physical addresses that mapped by no local access window
> +   (LAW) or an invalid LAW, as well as bad cache state that
> +   represents a coherency violation.
> +
>  config FSL_IFC
>   bool
>   depends on FSL_SOC
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index
> 71160a2..4055c47 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_OF)  += of_memory.o
>  endif
>  obj-$(CONFIG_TI_AEMIF)   += ti-aemif.o
>  obj-$(CONFIG_TI_EMIF)+= emif.o
> +obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o
>  obj-$(CONFIG_FSL_IFC)+= fsl_ifc.o
>  obj-$(CONFIG_MVEBU_DEVBUS)   += mvebu-devbus.o
>  obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
> diff --git a/drivers/memory/fsl-corenet-cf.c b/drivers/memory/fsl-corenet-cf.c
> new file mode 100644 index 000..a57a614
> --- /dev/null
> +++ b/drivers/memory/fsl-corenet-cf.c
> @@ -0,0 +1,246 @@
> +/*
> + * CoreNet Coherency Fabric error reporting
> + *
> + * Copyright 2014 Freescale Semiconductor Inc.
> + *
> + * This program is free software; you can redistribute  it and/or
> +modify it
> + * under  the terms of  the GNU General  Public License as published by
> +the
> + * Free Software Foundation;  either version 2 of the  License, or (at
> +your
> + * option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +enum ccf_version {
> + CCF1,
> + CCF2,
> +};
> +
> +struct ccf_info {
> + enum ccf_version version;
> + int err_reg_offs;
> +};
> +
> +static const struct ccf_info ccf1_info = {
> + .version = CCF1,
> + .err_reg_offs = 0xa00,
> +};
> +
> +static const struct ccf_info ccf2_info = {
> + .version = CCF2,
> + .err_reg_offs = 0xe40,
> +};
> +
> +static const struct of_device_id ccf_matches[] = {
> + {
> + .compatible = "fsl,corenet1-cf",
> + .data = _info,
> + },
> + {
> + .compatible = "fsl,corenet2-cf",
> + .data = _info,
> + },
> + {}
> +};
> +
> +struct ccf_err_regs {
> + u32 errdet; /* 0x00 Error Detect Register */
> + /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
> + u32 errdis;
> + /* 0x08 Error Interrupt Enable Register (ccf2 only) */
> +  

RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread bharat.bhus...@freescale.com


 -Original Message-
 From: Linuxppc-dev [mailto:linuxppc-dev-
 bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of Scott Wood
 Sent: Saturday, May 31, 2014 3:58 AM
 To: Greg Kroah-Hartman
 Cc: linuxppc-...@lists.ozlabs.org; linux-kernel@vger.kernel.org
 Subject: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
 reporting driver
 
 The CoreNet Coherency Fabric is part of the memory subsystem on some Freescale
 QorIQ chips.  It can report coherency violations (e.g.
 due to misusing memory that is mapped noncoherent) as well as transactions 
 that
 do not hit any local access window, or which hit a local access window with an
 invalid target ID.
 
 Signed-off-by: Scott Wood scottw...@freescale.com
 ---
 Resending to the proper list addresses -- sorry for the duplicate.
 
  arch/powerpc/configs/corenet32_smp_defconfig |   1 +
  arch/powerpc/configs/corenet64_smp_defconfig |   1 +
  drivers/memory/Kconfig   |  10 ++
  drivers/memory/Makefile  |   1 +
  drivers/memory/fsl-corenet-cf.c  | 246 
 +++
  5 files changed, 259 insertions(+)
  create mode 100644 drivers/memory/fsl-corenet-cf.c
 
 diff --git a/arch/powerpc/configs/corenet32_smp_defconfig
 b/arch/powerpc/configs/corenet32_smp_defconfig
 index c19ff05..0c99d7e 100644
 --- a/arch/powerpc/configs/corenet32_smp_defconfig
 +++ b/arch/powerpc/configs/corenet32_smp_defconfig
 @@ -179,3 +179,4 @@ CONFIG_CRYPTO_SHA512=y  CONFIG_CRYPTO_AES=y  #
 CONFIG_CRYPTO_ANSI_CPRNG is not set  CONFIG_CRYPTO_DEV_FSL_CAAM=y
 +CONFIG_FSL_CORENET_CF=y
 diff --git a/arch/powerpc/configs/corenet64_smp_defconfig
 b/arch/powerpc/configs/corenet64_smp_defconfig
 index 5c7fa19..8fb616d 100644
 --- a/arch/powerpc/configs/corenet64_smp_defconfig
 +++ b/arch/powerpc/configs/corenet64_smp_defconfig
 @@ -175,3 +175,4 @@ CONFIG_CRYPTO_SHA256=y  CONFIG_CRYPTO_SHA512=y  #
 CONFIG_CRYPTO_ANSI_CPRNG is not set  CONFIG_CRYPTO_DEV_FSL_CAAM=y
 +CONFIG_FSL_CORENET_CF=y
 diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig index
 c59e9c9..fab81a1 100644
 --- a/drivers/memory/Kconfig
 +++ b/drivers/memory/Kconfig
 @@ -61,6 +61,16 @@ config TEGRA30_MC
 analysis, especially for IOMMU/SMMU(System Memory Management
 Unit) module.
 
 +config FSL_CORENET_CF
 + tristate Freescale CoreNet Error Reporting
 + depends on FSL_SOC_BOOKE
 + help
 +   Say Y for reporting of errors from the Freescale CoreNet
 +   Coherency Fabric.  Errors reported include accesses to
 +   physical addresses that mapped by no local access window
 +   (LAW) or an invalid LAW, as well as bad cache state that
 +   represents a coherency violation.
 +
  config FSL_IFC
   bool
   depends on FSL_SOC
 diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index
 71160a2..4055c47 100644
 --- a/drivers/memory/Makefile
 +++ b/drivers/memory/Makefile
 @@ -7,6 +7,7 @@ obj-$(CONFIG_OF)  += of_memory.o
  endif
  obj-$(CONFIG_TI_AEMIF)   += ti-aemif.o
  obj-$(CONFIG_TI_EMIF)+= emif.o
 +obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o
  obj-$(CONFIG_FSL_IFC)+= fsl_ifc.o
  obj-$(CONFIG_MVEBU_DEVBUS)   += mvebu-devbus.o
  obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
 diff --git a/drivers/memory/fsl-corenet-cf.c b/drivers/memory/fsl-corenet-cf.c
 new file mode 100644 index 000..a57a614
 --- /dev/null
 +++ b/drivers/memory/fsl-corenet-cf.c
 @@ -0,0 +1,246 @@
 +/*
 + * CoreNet Coherency Fabric error reporting
 + *
 + * Copyright 2014 Freescale Semiconductor Inc.
 + *
 + * This program is free software; you can redistribute  it and/or
 +modify it
 + * under  the terms of  the GNU General  Public License as published by
 +the
 + * Free Software Foundation;  either version 2 of the  License, or (at
 +your
 + * option) any later version.
 + */
 +
 +#include linux/interrupt.h
 +#include linux/io.h
 +#include linux/irq.h
 +#include linux/module.h
 +#include linux/of.h
 +#include linux/of_address.h
 +#include linux/of_device.h
 +#include linux/of_irq.h
 +#include linux/platform_device.h
 +
 +enum ccf_version {
 + CCF1,
 + CCF2,
 +};
 +
 +struct ccf_info {
 + enum ccf_version version;
 + int err_reg_offs;
 +};
 +
 +static const struct ccf_info ccf1_info = {
 + .version = CCF1,
 + .err_reg_offs = 0xa00,
 +};
 +
 +static const struct ccf_info ccf2_info = {
 + .version = CCF2,
 + .err_reg_offs = 0xe40,
 +};
 +
 +static const struct of_device_id ccf_matches[] = {
 + {
 + .compatible = fsl,corenet1-cf,
 + .data = ccf1_info,
 + },
 + {
 + .compatible = fsl,corenet2-cf,
 + .data = ccf2_info,
 + },
 + {}
 +};
 +
 +struct ccf_err_regs {
 + u32 errdet; /* 0x00 Error Detect Register */
 + /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
 + u32 errdis;
 + /* 0x08 Error Interrupt Enable Register 

Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread Scott Wood
On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
  +struct ccf_err_regs {
  +   u32 errdet; /* 0x00 Error Detect Register */
  +   /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
  +   u32 errdis;
  +   /* 0x08 Error Interrupt Enable Register (ccf2 only) */
  +   u32 errinten;
  +   u32 cecar;  /* 0x0c Error Capture Attribute Register */
  +   u32 cecadrh;/* 0x10 Error Capture Address High */
 
 s/cecadrh/cecaddrh/g
 This way we will be consistent with Reference manual.

It's cecadrh in ccf1 and cecaddrh in ccf2.  I suppose I should use
the latter since errdet/errdis/errinten are the ccf2 names.

  +   u32 cecadrl;/* 0x14 Error Capture Address Low */
 
 s/cecadrl/cecaddrl/g
 
  +   u32 cecar2; /* 0x18 Error Capture Attribute Register 2 */
  +};
  +
  +/* LAE/CV also valid for errdis and errinten */
  +#define ERRDET_LAE (1  0)  /* Local Access Error */
  +#define ERRDET_CV  (1  1)  /* Coherency Violation */
  +#define ERRDET_CTYPE_SHIFT 26/* Capture Type (ccf2 only) */
  +#define ERRDET_CTYPE_MASK  (0x3f  ERRDET_CTYPE_SHIFT)
 
 Should not this be (0x1f  ERRDET_CTYPE_SHIFT)

Yes, thanks for catching that.

  +#define ERRDET_CAP (1  31) /* Capture Valid (ccf2 only) */
  +
  +#define CECAR_VAL  (1  0)  /* Valid (ccf1 only) */
  +#define CECAR_UVT  (1  15) /* Unavailable target ID (ccf1) */
  +#define CECAR_SRCID_SHIFT_CCF1 24
  +#define CECAR_SRCID_MASK_CCF1  (0xff  CECAR_SRCID_SHIFT_CCF1)
  +#define CECAR_SRCID_SHIFT_CCF2 18
  +#define CECAR_SRCID_MASK_CCF2  (0xff  CECAR_SRCID_SHIFT_CCF2)
  +
  +#define CECADRH_ADDRH  0xf
 
 On ccf2 this id 0xff.

OK.  I think we can get away with using 0xff on both.

  +static int ccf_remove(struct platform_device *pdev) {
  +   struct ccf_private *ccf = dev_get_drvdata(pdev-dev);
  +
  +   switch (ccf-info-version) {
  +   case CCF1:
  +   iowrite32be(0, ccf-err_regs-errdis);
  +   break;
  +
  +   case CCF2:
  +   iowrite32be(0, ccf-err_regs-errinten);
 
 Do you think it is same to disable detection bits in ccf-err_regs-errdis?

Disabling the interrupt is what we're aiming for here, but ccf1 doesn't
provide a way to do that separate from disabling detection.

-Scott


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread bharat.bhus...@freescale.com


 -Original Message-
 From: Wood Scott-B07421
 Sent: Wednesday, June 04, 2014 10:12 PM
 To: Bhushan Bharat-R65777
 Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
 ker...@vger.kernel.org
 Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
 reporting driver
 
 On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
   +struct ccf_err_regs {
   + u32 errdet; /* 0x00 Error Detect Register */
   + /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
   + u32 errdis;
   + /* 0x08 Error Interrupt Enable Register (ccf2 only) */
   + u32 errinten;
   + u32 cecar;  /* 0x0c Error Capture Attribute Register */
   + u32 cecadrh;/* 0x10 Error Capture Address High */
 
  s/cecadrh/cecaddrh/g
  This way we will be consistent with Reference manual.
 
 It's cecadrh in ccf1 and cecaddrh in ccf2.  I suppose I should use the
 latter since errdet/errdis/errinten are the ccf2 names.
 
   + u32 cecadrl;/* 0x14 Error Capture Address Low */
 
  s/cecadrl/cecaddrl/g
 
   + u32 cecar2; /* 0x18 Error Capture Attribute Register 2 */
   +};
   +
   +/* LAE/CV also valid for errdis and errinten */
   +#define ERRDET_LAE   (1  0)  /* Local Access Error */
   +#define ERRDET_CV(1  1)  /* Coherency Violation */
   +#define ERRDET_CTYPE_SHIFT   26/* Capture Type (ccf2 only) */
   +#define ERRDET_CTYPE_MASK(0x3f  ERRDET_CTYPE_SHIFT)
 
  Should not this be (0x1f  ERRDET_CTYPE_SHIFT)
 
 Yes, thanks for catching that.
 
   +#define ERRDET_CAP   (1  31) /* Capture Valid (ccf2 only) 
   */
   +
   +#define CECAR_VAL(1  0)  /* Valid (ccf1 only) */
   +#define CECAR_UVT(1  15) /* Unavailable target ID 
   (ccf1) */
   +#define CECAR_SRCID_SHIFT_CCF1   24
   +#define CECAR_SRCID_MASK_CCF1(0xff  CECAR_SRCID_SHIFT_CCF1)
   +#define CECAR_SRCID_SHIFT_CCF2   18
   +#define CECAR_SRCID_MASK_CCF2(0xff  CECAR_SRCID_SHIFT_CCF2)
   +
   +#define CECADRH_ADDRH0xf
 
  On ccf2 this id 0xff.
 
 OK.  I think we can get away with using 0xff on both.
 
   +static int ccf_remove(struct platform_device *pdev) {
   + struct ccf_private *ccf = dev_get_drvdata(pdev-dev);
   +
   + switch (ccf-info-version) {
   + case CCF1:
   + iowrite32be(0, ccf-err_regs-errdis);
   + break;
   +
   + case CCF2:
   + iowrite32be(0, ccf-err_regs-errinten);
 
  Do you think it is same to disable detection bits in ccf-err_regs-errdis?
 
 Disabling the interrupt is what we're aiming for here, but ccf1 doesn't 
 provide
 a way to do that separate from disabling detection.

What I wanted to say that do we also need to disable detection (set ERRDET_LAE 
| ERRDET_CV bits in errdis) apart from clearing errinten on ccf2 ?

Thanks
-Bharat

 
 -Scott
 



Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error reporting driver

2014-06-04 Thread Scott Wood
On Wed, 2014-06-04 at 12:04 -0500, Bhushan Bharat-R65777 wrote:
 
  -Original Message-
  From: Wood Scott-B07421
  Sent: Wednesday, June 04, 2014 10:12 PM
  To: Bhushan Bharat-R65777
  Cc: Greg Kroah-Hartman; linuxppc-...@lists.ozlabs.org; linux-
  ker...@vger.kernel.org
  Subject: Re: [RESEND PATCH] memory: Freescale CoreNet Coherency Fabric error
  reporting driver
  
  On Wed, 2014-06-04 at 03:17 -0500, Bhushan Bharat-R65777 wrote:
+static int ccf_remove(struct platform_device *pdev) {
+   struct ccf_private *ccf = dev_get_drvdata(pdev-dev);
+
+   switch (ccf-info-version) {
+   case CCF1:
+   iowrite32be(0, ccf-err_regs-errdis);
+   break;
+
+   case CCF2:
+   iowrite32be(0, ccf-err_regs-errinten);
  
   Do you think it is same to disable detection bits in 
   ccf-err_regs-errdis?
  
  Disabling the interrupt is what we're aiming for here, but ccf1 doesn't 
  provide
  a way to do that separate from disabling detection.
 
 What I wanted to say that do we also need to disable detection (set
 ERRDET_LAE | ERRDET_CV bits in errdis) apart from clearing errinten on
 ccf2 ?

I don't think we need to.  You could argue that we should for
consistency, though I think there's value in errors continuing to be
detected even without the driver (e.g. can dump the registers in a
debugger).

-Scott


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/