All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Grant Likely" <grant.likely@secretlab.ca>
To: avorontsov@ru.mvista.com
Cc: David Brownell <dbrownell@users.sourceforge.net>,
	Gary Jennejohn <garyj@denx.de>,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	Guennadi Liakhovetski <g.liakhovetski@gmx.de>,
	Pierre Ossman <drzeus-mmc@drzeus.cx>
Subject: Re: [PATCH] [RFC v3] OF: OpenFirmware bindings for the mmc_spi driver
Date: Thu, 5 Jun 2008 12:42:49 -0600	[thread overview]
Message-ID: <fa686aa40806051142q61ba471fl772ed32586b70e5c@mail.gmail.com> (raw)
In-Reply-To: <20080605183146.GA29092@polina.dev.rtsoft.ru>

On Thu, Jun 5, 2008 at 12:31 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Thu, Jun 05, 2008 at 12:18:56PM -0600, Grant Likely wrote:
>> On Thu, Jun 5, 2008 at 12:00 PM, Anton Vorontsov
>> <avorontsov@ru.mvista.com> wrote:
>> > On Thu, Jun 05, 2008 at 11:36:09AM -0600, Grant Likely wrote:
>> >> On Thu, Jun 5, 2008 at 11:27 AM, Anton Vorontsov
>> >> <avorontsov@ru.mvista.com> wrote:
>> >> > Well, I mentioned the usb_add_hcd()-alike approach for the mmc_spi
>> >> > host... The absence of enthusiasm I equaled to "no".
>> >> >
>> >> > Heh.
>> >>
>> >> I'm allergic to USB HCD code; I was probably having convulsions under my desk.
>> >
>> > :-)
>> >
>> > Ok, I also mentioned drivers/ata/pata_of_platform.c (OF version is using
>> > common code from drivers/ata/pata_platform.c).
>> >
>> > Please look there, and tell me if this is what you have in mind. (ignore
>> > _probe in the __pata_platform_probe name. Imagine
>> > pata_platform_add_controller or something).
>>
>> Yes, I like that.  I've done something very similar for drivers with
>> both of and non-of bindings.  For another example, this time all
>> contained within a single .c file, see drivers/video/xilinxfb.c
>
> Ok, great. As I said previously, this is quite easy to do.
>
>> >> > p.s.
>> >> > Btw, you forgot another downside of v2 approach: struct spi_driver
>> >> > duplication... Not sure if everyone will be happy about it.
>> >> >
>> >> > Though, v2 is only version where we can make modular OF_MMC_SPI.
>> >>
>> >> I think we've got our wires crossed.  I'm not referring to the option
>> >> of an of_mmc_spi driver registering an mmc_spi device (which can then
>> >> be probed by the mmc_spi_driver).
>> >
>> > I'm not refrering to this option either.
>>
>> Okay, I'm confused then.  Where is the duplication of struct spi_driver?
>
> Here it is http://lkml.org/lkml/2008/5/23/299
> + static struct spi_driver of_mmc_spi_driver = {

Right; I was going down the wrong thought path.  I have no problem with this.

BTW, while on that topic, I think it is reasonable to roll the members
of of_mmc_spi into either the mmc_spi_platform_data or the
mmc_spi_host structure.  It is just 2 integers and that would
eliminate storing driver data pointers in seemingly random places.

> And here http://lkml.org/lkml/2008/5/24/153 David Brownell says:
>> The only thing that looks odd to me about this is that the wrapper
>> is a spi_device rather than an of_device.  To me it makes more sense
>> to just have an of_device setting up the right spi_device.  (Though
>> maybe I missed some discussion about why that can't work.)

Yeah; I'm not fond of that approach.  It incurs runtime cost of
multiple 'struct device' for a single device which is unnecessary.

> I hope the bottom line is that we're now all happy to create another
> spi_driver to handle "OF MMC-o-SPI" devices..?

Yes, I'm cool with it.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

WARNING: multiple messages have this Message-ID (diff)
From: "Grant Likely" <grant.likely@secretlab.ca>
To: avorontsov@ru.mvista.com
Cc: "Pierre Ossman" <drzeus-mmc@drzeus.cx>,
	"David Brownell" <dbrownell@users.sourceforge.net>,
	"Jochen Friedrich" <jochen@scram.de>,
	"Segher Boessenkool" <segher@kernel.crashing.org>,
	"Gary Jennejohn" <garyj@denx.de>,
	"Guennadi Liakhovetski" <g.liakhovetski@gmx.de>,
	linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [RFC v3] OF: OpenFirmware bindings for the mmc_spi driver
Date: Thu, 5 Jun 2008 12:42:49 -0600	[thread overview]
Message-ID: <fa686aa40806051142q61ba471fl772ed32586b70e5c@mail.gmail.com> (raw)
In-Reply-To: <20080605183146.GA29092@polina.dev.rtsoft.ru>

On Thu, Jun 5, 2008 at 12:31 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Thu, Jun 05, 2008 at 12:18:56PM -0600, Grant Likely wrote:
>> On Thu, Jun 5, 2008 at 12:00 PM, Anton Vorontsov
>> <avorontsov@ru.mvista.com> wrote:
>> > On Thu, Jun 05, 2008 at 11:36:09AM -0600, Grant Likely wrote:
>> >> On Thu, Jun 5, 2008 at 11:27 AM, Anton Vorontsov
>> >> <avorontsov@ru.mvista.com> wrote:
>> >> > Well, I mentioned the usb_add_hcd()-alike approach for the mmc_spi
>> >> > host... The absence of enthusiasm I equaled to "no".
>> >> >
>> >> > Heh.
>> >>
>> >> I'm allergic to USB HCD code; I was probably having convulsions under my desk.
>> >
>> > :-)
>> >
>> > Ok, I also mentioned drivers/ata/pata_of_platform.c (OF version is using
>> > common code from drivers/ata/pata_platform.c).
>> >
>> > Please look there, and tell me if this is what you have in mind. (ignore
>> > _probe in the __pata_platform_probe name. Imagine
>> > pata_platform_add_controller or something).
>>
>> Yes, I like that.  I've done something very similar for drivers with
>> both of and non-of bindings.  For another example, this time all
>> contained within a single .c file, see drivers/video/xilinxfb.c
>
> Ok, great. As I said previously, this is quite easy to do.
>
>> >> > p.s.
>> >> > Btw, you forgot another downside of v2 approach: struct spi_driver
>> >> > duplication... Not sure if everyone will be happy about it.
>> >> >
>> >> > Though, v2 is only version where we can make modular OF_MMC_SPI.
>> >>
>> >> I think we've got our wires crossed.  I'm not referring to the option
>> >> of an of_mmc_spi driver registering an mmc_spi device (which can then
>> >> be probed by the mmc_spi_driver).
>> >
>> > I'm not refrering to this option either.
>>
>> Okay, I'm confused then.  Where is the duplication of struct spi_driver?
>
> Here it is http://lkml.org/lkml/2008/5/23/299
> + static struct spi_driver of_mmc_spi_driver = {

Right; I was going down the wrong thought path.  I have no problem with this.

BTW, while on that topic, I think it is reasonable to roll the members
of of_mmc_spi into either the mmc_spi_platform_data or the
mmc_spi_host structure.  It is just 2 integers and that would
eliminate storing driver data pointers in seemingly random places.

> And here http://lkml.org/lkml/2008/5/24/153 David Brownell says:
>> The only thing that looks odd to me about this is that the wrapper
>> is a spi_device rather than an of_device.  To me it makes more sense
>> to just have an of_device setting up the right spi_device.  (Though
>> maybe I missed some discussion about why that can't work.)

Yeah; I'm not fond of that approach.  It incurs runtime cost of
multiple 'struct device' for a single device which is unnecessary.

> I hope the bottom line is that we're now all happy to create another
> spi_driver to handle "OF MMC-o-SPI" devices..?

Yes, I'm cool with it.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

  reply	other threads:[~2008-06-05 18:42 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-05 16:16 [PATCH] [RFC v3] OF: OpenFirmware bindings for the mmc_spi driver Anton Vorontsov
2008-06-05 16:16 ` Anton Vorontsov
2008-06-05 16:45 ` Grant Likely
2008-06-05 16:45   ` Grant Likely
2008-06-05 17:27   ` Anton Vorontsov
2008-06-05 17:27     ` Anton Vorontsov
2008-06-05 17:36     ` Grant Likely
2008-06-05 17:36       ` Grant Likely
2008-06-05 18:00       ` Anton Vorontsov
2008-06-05 18:00         ` Anton Vorontsov
2008-06-05 18:18         ` Grant Likely
2008-06-05 18:18           ` Grant Likely
2008-06-05 18:31           ` Anton Vorontsov
2008-06-05 18:31             ` Anton Vorontsov
2008-06-05 18:42             ` Grant Likely [this message]
2008-06-05 18:42               ` Grant Likely
2008-06-14 15:57 ` Pierre Ossman
2008-06-16 13:23   ` Anton Vorontsov
2008-06-16 13:23     ` Anton Vorontsov
2008-06-16 14:13   ` Grant Likely
2008-06-16 14:13     ` Grant Likely
2008-07-03  3:26 ` Jon Smirl
2008-07-03  3:26   ` Jon Smirl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fa686aa40806051142q61ba471fl772ed32586b70e5c@mail.gmail.com \
    --to=grant.likely@secretlab.ca \
    --cc=avorontsov@ru.mvista.com \
    --cc=dbrownell@users.sourceforge.net \
    --cc=drzeus-mmc@drzeus.cx \
    --cc=g.liakhovetski@gmx.de \
    --cc=garyj@denx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.