Re: [PATCH v2 1/3] mmc: add Coldfire esdhc support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Adrian,

On Tue, Jul 02, 2019 at 12:10:54PM +0300, Adrian Hunter wrote:
> On 20/06/19 1:22 AM, Angelo Dureghello wrote:
> > Hi Christoph,
> > 
> > On Sun, Jun 16, 2019 at 11:58:07PM -0700, Christoph Hellwig wrote:
> >> On Sun, Jun 16, 2019 at 10:48:21PM +0200, Angelo Dureghello wrote:
> >>> This driver has been developed as a separate module starting
> >>> from the similar sdhci-esdhc-fls.c.
> >>> Separation has been mainly driven from change in endianness.
> >>
> >> Can't we have a way to define the endianess at build or even runtime?
> >> We have plenty of that elsewhere in the kernel.
> > 
> > well, the base sdhci layer wants to access byte-size fields of the
> > esdhc controller registers.
> > But this same Freescale esdhc controller may be found in 2
> > flavors, big-endian or little-endian organized.
> > So in this driver i am actually correcting byte-addresses to
> > access the wanted byte-field in the big-endian hw controller.
> > 
> > So this is a bit different from a be-le endian swap of a
> > long or a short that the kernel is organized to do..
> 
> Did you consider just using different sdhci_ops so that you could support
> different sdhci I/O accessors?

Initially i tried to modify the IMX/Vybrid (arm) driver. But was stopped from
several points, trying to remember now, 

- the I/O accessors was a const struct, but this of course is not a big 
  issue,
- here and there bitfields and positions of the ColdFire controller
  registers, compared to the arm versions, are different, so controller hw
  is not exactly the same,
- on ColdFire controller and some behaviors and errata are different,
- dma endiannes not hw-configurable,
- ColdFire has max clock limitations, a bit different clock init.

Finally, since there is already a common library (shdci.c) i decided to go
as a separate driver, instead of filling the driver of "if (coldfire)" also 
mainly for the following reasons:

1) separated ColdFire driver has a quite small amount of code, simple to
understand.
2) having drivers used by multiple architectures, it add risks, each time
i perform a change, i can test it only on ColdFire, and can break
the driver for the other architectures (i see this not rarely happening for
multiple-arch used drivers).

So let me know if the way chosen can be ok. Otherwise i will roll back 
trying to modify the iMX/Vybrid driver, likely adding a lot of "if (coldfire)"
complicating it quite a lot.

Regards,
Angelo



[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux