Re: CVS commit: src/sys/dev/sdmmc

[Matt Thomas <matt%3am-software.com@localhost>]

On Jul 17, 2012, at 5:39 AM, Greg Troxel wrote:

> 
>  Modified Files:
>          src/sys/dev/sdmmc: sdhc.c
> 
>  Log Message:
>  Handle interrupt acknowledgement in the SDHC_FLAG_32BIT_ACCESS case in
>  the same way as non-SDHC_FLAG_32BIT_ACCESS case.
> 
>  To generate a diff of this commit:
>  cvs rdiff -u -r1.20 -r1.21 src/sys/dev/sdmmc/sdhc.c
> 
> Two questions, one of which isn't about your change.
> 
> It seems that the HREAD4 of SDHC_NINTR_STATUS is reading both that and
> SDHC_EINTR_STATUS, but it seems odd that those are addresses 0x30 and
> 0x32, which don't seem 4-byte aligned.  So is it really ok to do an
> HREAD4 across boundaries like that?  It seems odd, but maybe it's about
> the bus and the chip is fine with it.

0x30 is aligned and that is the address that is read from a 32-bit acccess.

> In the non-32 case, it seems that the EINTR register is read and then
> written back exactly if the error bit is set in the NINTR register.  In
> the 32 case, it seems that the SDHC_ERROR_INTERRUPT bit is set in NINTR
> if any bits are set in EINTR, in addition to writing both.
> 
> So while I see the point that the |= into status is effectively a dead
> store,
> 
>  it seems odd to have bits set in SDHC_EINTR_STATUS without
>  SDHC_ERROR_INTERRUPT set in SDHC_NINTR_STATUS, and
>  the two code paths seem different still

This is because the ESDHC doesn't set SDHC_ERROR_INTERRUPT in its
register since you've read SDHC_EINTR_STATUS and can see those bits
directly.  So, for the sdhc driver, it needs to be emulated.

> I'm curious if you've been seeing bits in EINTR without ERROR in NINTR,
> and what the symptoms are that provoked this fix - I have a system with
> an sdhc (on evbppc/P2020) that mostly works but has occasional write
> errors.

That is exactly what the ESDHC on the P2020 does.