Re: [PATCH 3/5] mmc: sdhi: Add write16_hook

Simon Horman
2011-06-21T00:50:30Z

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@verge.net.au> 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@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> >
> > 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 *.