2009-06-01 23:55:47

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

On Sat, 30 May 2009 20:05:52 +0200
Tobias M__ller <[email protected]> wrote:

> From: Tobias Mueller <[email protected]>
>
> A GPIO driver for AMD Geode Companion Device CS5535/CS5536
> using the GPIO framework as a replacement for old cs5535-gpio driver.
> Signed-off-by: Tobias Mueller <[email protected]>

Last time, I said "We'll need to discuss the migration plan - we don't
want to maintain two drivers for the same thing".

Back in February, Andres sent out a patch "cs553x-gpio: add AMD
CS5535/CS5536 GPIO driver support". AFACIT that patch does the same
thing that this one does, but differently. IIRC it had a great string
of weird-Kconfig-related build errors and I eventually dropped it.

What's the story here?

Thanks.


2009-06-02 00:32:30

by Andres Salomon

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

On Mon, 1 Jun 2009 16:54:57 -0700
Andrew Morton <[email protected]> wrote:
[...]
>
> Back in February, Andres sent out a patch "cs553x-gpio: add AMD
> CS5535/CS5536 GPIO driver support". AFACIT that patch does the same
> thing that this one does, but differently. IIRC it had a great string
> of weird-Kconfig-related build errors and I eventually dropped it.
>
> What's the story here?
>
> Thanks.

My story is that I've since switched employers twice since Feb, and will
be resubmitting the patch now that sanity has returned to my life.
I'm planning to get to it within the next week.

As far as Tobias's driver, one major difference between our drivers is
that mine doesn't grab the entire PCI device. This is because other
drivers (ie, the MFGPT driver) will want to share the PCI device.

2009-06-02 03:10:12

by Jordan Crouse

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

Andres Salomon wrote:
> On Mon, 1 Jun 2009 16:54:57 -0700
> Andrew Morton <[email protected]> wrote:
> [...]
>> Back in February, Andres sent out a patch "cs553x-gpio: add AMD
>> CS5535/CS5536 GPIO driver support". AFACIT that patch does the same
>> thing that this one does, but differently. IIRC it had a great string
>> of weird-Kconfig-related build errors and I eventually dropped it.
>>
>> What's the story here?
>>
>> Thanks.
>
> My story is that I've since switched employers twice since Feb, and will
> be resubmitting the patch now that sanity has returned to my life.
> I'm planning to get to it within the next week.
>
> As far as Tobias's driver, one major difference between our drivers is
> that mine doesn't grab the entire PCI device. This is because other
> drivers (ie, the MFGPT driver) will want to share the PCI device.

Also SMBUS. In fact, they *must* share the PCI device if they all want
to play well together, so that alone would probably push us toward
Andre's patch, once it gets clean.

Jordan

2009-06-02 08:14:57

by Tobias Müller

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

2009/6/2 Jordan Crouse <[email protected]>:
> Andres Salomon wrote:
>> As far as Tobias's driver, one major difference between our drivers is
>> that mine doesn't grab the entire PCI device. This is because other
>> drivers (ie, the MFGPT driver) will want to share the PCI device.
>
> Also SMBUS. In fact, they *must* share the PCI device if they all want to
> play well together, so that alone would probably push us toward Andre's
> patch, once it gets clean.

I had SMBUS and my driver tested in parallel and there I didn't found any
problems. But I would be fine if we would use Andre's patch. I haven't know
about, so that's why I tried my own. :)

I see some other problem's on Andre's patch:
- He allows the use of PIN 23 which is reserved
- No special handling is done for 28, which is a GPIO, but
serves as Power Button as well
- OUT_AUX1/OUT_AUX2 should be disabled by default
if pin is configured as in output
- IN_AUX should be disabled by default is pin is
configured as in input

And, if I request a GPIO by userspace via sysfs, on my system it's an
input pin by default. So my driver initializes a GPIO as an input by default.
But I don't know if this is default or system specific. I didn't found anything
about this in the gpio docs.

So maybe we should decide whether we take Andre's or my approach
and then adjust it.

Tobias

2009-06-02 20:33:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

On Tue, 2 Jun 2009 10:14:45 +0200
Tobias M__ller <[email protected]> wrote:

> 2009/6/2 Jordan Crouse <[email protected]>:
> > Andres Salomon wrote:
> >> As far as Tobias's driver, one major difference between our drivers is
> >> that mine doesn't grab the entire PCI device. This is because other
> >> drivers (ie, the MFGPT driver) will want to share the PCI device.
> >
> > Also SMBUS. In fact, they *must* share the PCI device if they all want to
> > play well together, so that alone would probably push us toward Andre's
> > patch, once it gets clean.
>
> I had SMBUS and my driver tested in parallel and there I didn't found any
> problems. But I would be fine if we would use Andre's patch. I haven't know
> about, so that's why I tried my own. :)
>
> I see some other problem's on Andre's patch:
> - He allows the use of PIN 23 which is reserved
> - No special handling is done for 28, which is a GPIO, but
> serves as Power Button as well
> - OUT_AUX1/OUT_AUX2 should be disabled by default
> if pin is configured as in output
> - IN_AUX should be disabled by default is pin is
> configured as in input
>
> And, if I request a GPIO by userspace via sysfs, on my system it's an
> input pin by default. So my driver initializes a GPIO as an input by default.
> But I don't know if this is default or system specific. I didn't found anything
> about this in the gpio docs.
>
> So maybe we should decide whether we take Andre's or my approach
> and then adjust it.
>

I think I'll drop gpio-amd-cs5535-cs5536-gpio-driver.patch while you
guys sort all this out, please.

2009-06-09 12:27:35

by Tobias Müller

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

>> So maybe we should decide whether we take Andre's or my approach
>> and then adjust it.
>>
>
> I think I'll drop gpio-amd-cs5535-cs5536-gpio-driver.patch while you
> guys sort all this out, please.

So, how to carry on with this? Shall I adapt Andre's version to mine or
will Andre take care of this version?

A comment from Andre would be fine.

2009-06-10 04:00:50

by Andres Salomon

[permalink] [raw]
Subject: Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

On Tue, 9 Jun 2009 14:27:30 +0200
Tobias Müller <[email protected]> wrote:

> >> So maybe we should decide whether we take Andre's or my approach
> >> and then adjust it.
> >>
> >
> > I think I'll drop gpio-amd-cs5535-cs5536-gpio-driver.patch while you
> > guys sort all this out, please.
>
> So, how to carry on with this? Shall I adapt Andre's version to mine
> or will Andre take care of this version?
>
> A comment from Andre would be fine.


My preference would be for you to base your work on the patches that
I'm about to (re)submit. I now actually have time to get these through
the queue.