On Thu, Oct 07, 2010 at 06:00:06PM +0200, Juergen Beisert wrote:
> Sascha Hauer wrote:
> > On Thu, Oct 07, 2010 at 03:24:16PM +0200, Juergen Beisert wrote:
> > > This adds the basic framework to handle MCI cards in barebox.
> > >
> > > Signed-off-by: Juergen Beisert <jbe@xxxxxxxxxxxxxx>
> > > ---
> > > drivers/Kconfig | 1 +
> > > drivers/Makefile | 1 +
> > > drivers/mci/Kconfig | 30 ++
> > > drivers/mci/Makefile | 1 +
> > > drivers/mci/mci-core.c | 1308
> > > ++++++++++++++++++++++++++++++++++++++++++++++++ include/mci.h |
> > > 230 +++++++++
> > > 6 files changed, 1571 insertions(+), 0 deletions(-)
> > > create mode 100644 drivers/mci/Kconfig
> > > create mode 100644 drivers/mci/Makefile
> > > create mode 100644 drivers/mci/mci-core.c
> > > create mode 100644 include/mci.h
> >
> > This whole patch looks quite good.
> > Please add some linebreaks in mci-core.c. I don't want strict 80
> > character lines, but some lines are really long.
> >
> > [snip]
> >
> > > +static int mci_probe(struct device_d *mci_dev)
> > > +{
> > > + struct mci *mci;
> > > + int rc;
> > > +
> > > + mci = xzalloc(sizeof(struct mci));
> > > + mci_dev->priv = mci;
> > > +
> > > +#ifdef CONFIG_MCI_STARTUP
> > > + /* if enabled, probe the attached card immediately */
> > > + rc = mci_card_probe(mci_dev);
> > > + if (rc == -ENODEV) {
> > > + /*
> > > + * If it fails, add the 'probe' parameter to give the user
> > > + * a chance to insert a card and try again. Note: This may fail
> > > + * systems that rely on the MCI card for startup (for the
> > > + * persistant environment for example)
> > > + */
> > > + rc = add_mci_parameter(mci_dev);
> > > + if (rc != 0) {
> > > + pr_err("Failed to add 'probe' parameter to the MCI device\n");
> > > + goto on_error;
> > > + }
> > > + }
> > > +#endif
> > > +
> > > +#ifndef CONFIG_MCI_STARTUP
> >
> > #else instead?
> >
> > > + /* add params on demand */
> > > + rc = add_mci_parameter(mci_dev);
> > > + if (rc != 0) {
> > > + pr_err("Failed to add 'probe' parameter to the MCI device\n");
> > > + goto on_error;
> > > + }
> > > +#endif
> > > +
> > > + return rc;
> > > +
> > > +on_error:
> > > + free(mci);
> > > + return rc;
> > > +}
> > > +
> >
> > [snip]
> >
> > > +
> > > +/** host information */
> > > +struct mci_platformdata {
> > > + struct device_d *hw_dev; /**< the host MCI hardware device */
> > > + unsigned voltages;
> > > + unsigned host_caps; /**< Host's interface capabilities, refer MMC_VDD_*
> > > and FIXME */ + unsigned f_min; /**< host interface lower limit */
> > > + unsigned f_max; /**< host interface upper limit */
> > > + unsigned clock; /**< Current clock used to talk to the card */
> > > + unsigned bus_width; /**< used data bus width to the card */
> > > +
> > > + int (*init)(struct device_d*, struct device_d*); /**< init the host
> > > interface */ + void (*set_ios)(struct device_d*, struct device_d*,
> > > unsigned, unsigned); /**< change host interface settings */ + int
> > > (*send_cmd)(struct device_d*, struct mci_cmd*, struct mci_data*); /**<
> > > handle a command */ +};
> >
> > I prefer this struct named mci_host, this seems to match better what it
> > actually is.
>
> Hmm, no, its not a "host". It is the MMC/SD card instance. The host is more
> the interface, isn't it (at least for me)?
It's a host at least in the Linux terminology.
>
> > For the convenience of drivers set init/set_ios/send_cmd
> > functions should be passed a pointer to the mci_host, not the device,
> > because that's what they actually registered. I already prepared a patch
> > for this, I'll send it in a seperate mail.
>
> I tried to layering the devices:
>
> disk_device -> knows how to handle disk drives and partition tables
> |
> mci_device -> knows how to probe and manage MMC/SD cards
> |
> hw_device -> knows how to transfer data
>
> Most functions in the hw_device layer do not need access to any other
> structure data than their own device (okay, there is a exception). So, I
> tried to keep it simple.
I didn't change the underlying structure, only the pointer which is
passed to the functions.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
