[PATCH 15/23] Alternative mmc structure to support pxa168, pxa910, mmp2 family SD

quoted

On Fri, Dec 31, 2010 at 5:08 PM, Mark Brown [off-list ref] wrote:
On Dec 31, 2010, at 1:46 AM, Haojian Zhuang wrote:

On Fri, Dec 31, 2010 at 2:03 PM, Philip Rakity [off-list ref] wrote:
On Dec 30, 2010, at 9:46 PM, Haojian Zhuang wrote:

On Thu, Dec 23, 2010 at 6:58 AM, Philip Rakity [off-list ref] wrote:
On Dec 22, 2010, at 6:10 AM, Arnd Bergmann wrote:

On Wednesday 22 December 2010 08:09:58 Philip Rakity wrote:
The PXA168, PXA910, and MMP2 are not the same SOC. ?The family
of embedded processors have slightly different internal blocks
for SD, I2C, etc. ?Sometimes it is important to know which SOC
is being used due to differences in the silicon. ?Sometimes it
is important to know evaluation boards should be selected based
on the SOC on the board.
This looks like you're moving in the wrong direction.

If the chips are just slightly different, you'd certainly
want to make sure that you can detect the difference at runtime,
and be able to use the same kernel on all of the variants.
MMP2 used PJ4 core --- PXA168/PXA910 use PJ1 so rather different architecture.

PXA168/PXA910 have slightly different internal peripherals with different quirks.
Certainly possible to tell this apart at runtime but not all peripherals are the same
and startup files ARE different.

MMP2 and PJ4 are different SoC silicons. But they're using similar SD
I think you meant MMP2 and PJ1 there. MMP2 is a PJ4 core.

IP, so we can share same driver to them. Different quirks can be
handled by different flags in run time.
Technically all of these SDHCI controllers are extremely similar. The point of the SDHCI-* family of drivers
are there to specify the implementation differences.

sdhci-pxa codebase is currently minimal it performs two operations:
1. Register sdhci instance using platform data.
2. Define common quirks.
3. Provides clock control callback

The PXA168/PXA910 (PJ1) versions would have specific needs:
1. There is a difference in clock control and power up sequencing
2. There is a specific I/O accessor needed to access registers
3. There are workarounds for SDIO that are needed.
4. There specific quirks needed for the PXA168/PXA910.
For mmp2 and pxa910, they are same ip in fact, same silicon bug, and
same workaround, and fixed together in updated version.
That's the reason they worked well in currently code base with same
quirk, same driver.
pxa168 may be different since no future stepping.

It is true, one is v2, the other is v3, but this is handled in sdhci.c
and transparent for specific driver.
The only difference is different private register, that may because
when define v2 and can not foresee v3 definition.
Still not find strong reason to add more specific driver only handle
different private register, if so more driver will be added for mmp2x,
for example, though same ip are used.

BTW, brownstone with mmc support is already added if work based on
pxa-linux-2.6.git, devel branch.

Even if we were to write a separate driver for PXA168/910 and MMP2 there would be no code duplication except for
the code to interpret the platform data.

In Philip's code he took the duplication into account and further abstracted it so that the platform data interpretation is done in
sdhci-pxa.c and the differences are exposed in the sdhci-pxa168 and sdhci-mmp2 modules.

There is already precedence for this already in sdhci-of-* implementation.

I am not sure why people think combining all of these differences into one massive sdhci-pxa driver would make maintenance simpler when the
relevant commonality is already abstracted away in sdhci.c.

The SD IP is not the same. ?One uses SD controller 3.0 and the other is 2.0.

They are the same in the sense that they public registers adhere to the appropriate SD spec 2.0/3.0 but
these accesses are handled by sdhci.c. ?(SD 3.0 extends the SD 2.0 spec by adding new registers and adding some bit definitions
in a compatible manner inside the public space).
Yes, you confirmed that they're same in the sense. It's enough. User
needn't care whether it's SD2.0 or SD3.0. It's the business of silicon
engineer.

The private registers that need to be programmed are not extensions but are at different locations and bit fields do not have exactly the same meaning.

The code to handle the differences is rather small and is not NOT placed in arch/arm/ directory but rather in the
drivers/mmc/host directory and follows the conventions to handle this.
In your patch, you divide them into silicon depedant files. I think
that putting them into arch/arm is better.

There's no reason to copy driver for each silicon

Instead, you promote each of the SOCs to a top-level family
in this patch, which makes it impossible to build a kernel
for more than one of them at a time.
That was the intent to handle the case of development board selection.
it is meaningless to select MMP2 development board with say PXA168 SoC.

Open to other way to handle this problem. ?Suggestions welcome.
Again, you did wrong. You couldn't make patch for top-level family. It
will introduce a lot of error to maintainers.
The patch selects ARCH-MMP (as now) but adds the ability
to also know which specific SoC was chosen. ?(This is sort of done by choosing the development board today).
If you want to change mmc code, push it into mmc tree. If you want to
change pxa code, push it into pxa tree. If they're dependant, please
make sure the sequence is right.

Your patches will make them mess.
suggest a solution.

The current mechanism of having the development board select the CPU does not seem right.
One can select a development board for MMP2 and PXA168 and yet the arch files to support each CPU
are different and not compatible. ?(for example cache handling).
Only one SoC can be installed on one board. If silicons are
pin-compatible to one board, we can register different boards. For
example, we can divide saarb as saarb_pv and saarb_mg.

If MMP2 and PXA168 are both installed on one board, it means that you
must run two kernel images on two APs. Actually, I don't think anyone
will design system like this way. If so, why not adopt the above
policy? You can divide the board into two sub-board on naming policy.
Thanks
Haojian

? ? ? Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help