On Tue, Jun 21, 2011 at 9:50 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> On Tue, Jun 21, 2011 at 09:36:03AM +0900, Magnus Damm wrote:
>> On Tue, Jun 21, 2011 at 8:00 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
>> > Some controllers require waiting for the bus to become idle
>> > before writing to some registers. I have implemented this
>> > by adding a hook to sd_ctrl_write16() and implementing
>> > a hook for SDHI which waits for the bus to become idle.
>> >
>> > Cc: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
>> > Cc: Magnus Damm <magnus.damm@xxxxxxxxx>
>> > Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
>> >
>> > ---
>> >
>> > Dependencies: "mmc: tmio: Share register access functions"
>> >
>> > v2:
>> > * Include linux/delay.h instead of asm/delay.h
>> > * Skip write if sh_mobile_sdhi_wait_idle() times out
>> > - The bus will probably be in an inconsistent state and writing
>> > may lock up the bus
>> > * Only set hook if TMIO_MMC_HAS_IDLE_WAIT is set in platform data
>> > rather than checking for TMIO_MMC_HAS_IDLE_WAIT each time the
>> > hook is called.
>> > ---
>>
>> Thanks Simon, this version looks much better!
>>
>> > index 5a90266..0dc9804 100644
>> > --- a/include/linux/mfd/tmio.h
>> > +++ b/include/linux/mfd/tmio.h
>> > @@ -94,6 +101,7 @@ struct tmio_mmc_data {
>> > void (*set_pwr)(struct platform_device *host, int state);
>> > void (*set_clk_div)(struct platform_device *host, int state);
>> > int (*get_cd)(struct platform_device *host);
>> > + int (*write16_hook)(struct tmio_mmc_host *host, int addr);
>> > };
>> >
>> > static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
>>
>> What's the reason behind passing "struct tmio_mmc_host *" as an
>> argument to the new hook? Performance? All other callbacks seem to
>> take a "struct platform_device *", so being consistent here may be
>> good unless it comes with too much overhead.
>
> The reason is that
> 1) The hook is called from sd_ctrl_write16 which takes
> struct tmio_mmc_host * as its first argument and;
> 2) The hook that has been implemented calls sd_ctrl_read16() which takes a
> struct tmio_mmc_host * as its first argument.
> So it seemed logical to pass that down.
>
> In the caes of 1) we can get the struct platform_device * using host->pdev.
> However, in the case of 2) is it less clear to me how we can get the
> struct tmio_mmc_host * from a struct platform_device *.
Have a look at the code in tmio_mmc_host_suspend() for some code that
does struct device * -> struct tmio_mmc_host *:
int tmio_mmc_host_suspend(struct device *dev)
{
struct mmc_host *mmc = dev_get_drvdata(dev);
struct tmio_mmc_host *host = mmc_priv(mmc);
You can easily change the dev_get_drvdata() to platform_get_drvdata(),
see include/linux/platform_device.h
I guess a similar conversion can be done in tmio_mmc_enable_dma() to
move from writew() to sd_ctrl_write16()?
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
