Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Sebastian Frias
Hi Sergei, On 03/23/2016 11:39 AM, Sergei Shtylyov wrote: >> gpiod_reset = devm_gpiod_get_optional(dev, "reset", >> GPIOD_OUT_HIGH); > >We shouldn't call _optional() then, should we? I could imagine the original intention was to be backward compatible. Indeed, if this call is not

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Sebastian Frias
Hi Sergei, On 03/23/2016 11:39 AM, Sergei Shtylyov wrote: >> gpiod_reset = devm_gpiod_get_optional(dev, "reset", >> GPIOD_OUT_HIGH); > >We shouldn't call _optional() then, should we? I could imagine the original intention was to be backward compatible. Indeed, if this call is not

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Sergei Shtylyov
Hello. On 3/23/2016 1:17 PM, Mason wrote: Preconditions: - Some of the devices a given driver handles have a reset line and others don't. - A non-empty subset (maybe all) of the devices that have a reset line require that this reset line is used. Then the way to handle this in the

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Sergei Shtylyov
Hello. On 3/23/2016 1:17 PM, Mason wrote: Preconditions: - Some of the devices a given driver handles have a reset line and others don't. - A non-empty subset (maybe all) of the devices that have a reset line require that this reset line is used. Then the way to handle this in the

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Mason
On 22/03/2016 20:42, Uwe Kleine-König wrote: > Preconditions: > - Some of the devices a given driver handles have a reset line and >others don't. > - A non-empty subset (maybe all) of the devices that have a reset line >require that this reset line is used. > > Then the way to handle

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Mason
On 22/03/2016 20:42, Uwe Kleine-König wrote: > Preconditions: > - Some of the devices a given driver handles have a reset line and >others don't. > - A non-empty subset (maybe all) of the devices that have a reset line >require that this reset line is used. > > Then the way to handle

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Sebastian Frias
Hi Uwe, On 03/22/2016 08:42 PM, Uwe Kleine-König wrote: > Hello Sebastian, > > On Tue, Mar 22, 2016 at 03:34:23PM +0100, Sebastian Frias wrote: >> I think we are in a deadlock :-) >> I'm going to reply inline below, but I will also send a different email >> to Daniel with a small recap. >> I

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-23 Thread Sebastian Frias
Hi Uwe, On 03/22/2016 08:42 PM, Uwe Kleine-König wrote: > Hello Sebastian, > > On Tue, Mar 22, 2016 at 03:34:23PM +0100, Sebastian Frias wrote: >> I think we are in a deadlock :-) >> I'm going to reply inline below, but I will also send a different email >> to Daniel with a small recap. >> I

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Uwe Kleine-König
Hello Sebastian, On Tue, Mar 22, 2016 at 03:34:23PM +0100, Sebastian Frias wrote: > I think we are in a deadlock :-) > I'm going to reply inline below, but I will also send a different email > to Daniel with a small recap. > I think he should share the intent of the "reset" mechanism he >

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Uwe Kleine-König
Hello Sebastian, On Tue, Mar 22, 2016 at 03:34:23PM +0100, Sebastian Frias wrote: > I think we are in a deadlock :-) > I'm going to reply inline below, but I will also send a different email > to Daniel with a small recap. > I think he should share the intent of the "reset" mechanism he >

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi, On 03/21/2016 09:41 PM, Uwe Kleine-König wrote: >>My patch basically gets rid of all this code. The thing that worries me >> is that the driver assumes that the reset singal is active low, despite what >> the GPIO specifier in the device tree has for the GPIO polarity. In fact, it >> will

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi, On 03/21/2016 09:41 PM, Uwe Kleine-König wrote: >>My patch basically gets rid of all this code. The thing that worries me >> is that the driver assumes that the reset singal is active low, despite what >> the GPIO specifier in the device tree has for the GPIO polarity. In fact, it >> will

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi Sergei, On 03/21/2016 09:15 PM, Sergei Shtylyov wrote: > >Do you have the PHY that requires the GPIO reset workaround? Unfortunately (or luckily :-) ) I don't have the faulty PHY, sorry. Best regards, Sebastian

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi Sergei, On 03/21/2016 09:15 PM, Sergei Shtylyov wrote: > >Do you have the PHY that requires the GPIO reset workaround? Unfortunately (or luckily :-) ) I don't have the faulty PHY, sorry. Best regards, Sebastian

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi Daniel, Would you mind commenting on this thread? Uwe and I are in a sort of deadlock because we each have a different understanding of the intention of your commit 13a56b449325. Basically the questions are: - What is the intention of 13a56b449325? - Did you mean for "reset" to be mandatory

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi Daniel, Would you mind commenting on this thread? Uwe and I are in a sort of deadlock because we each have a different understanding of the intention of your commit 13a56b449325. Basically the questions are: - What is the intention of 13a56b449325? - Did you mean for "reset" to be mandatory

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi Uwe, I think we are in a deadlock :-) I'm going to reply inline below, but I will also send a different email to Daniel with a small recap. I think he should share the intent of the "reset" mechanism he introduced, in particular if it is mandatory. On 03/21/2016 09:12 PM, Uwe Kleine-König

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-22 Thread Sebastian Frias
Hi Uwe, I think we are in a deadlock :-) I'm going to reply inline below, but I will also send a different email to Daniel with a small recap. I think he should share the intent of the "reset" mechanism he introduced, in particular if it is mandatory. On 03/21/2016 09:12 PM, Uwe Kleine-König

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sergei Shtylyov
On 03/21/2016 11:41 PM, Uwe Kleine-König wrote: Commit 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") introduced a dependency on GPIOLIB that was not there before. This commit removes such dependency by checking the return code and comparing it

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sergei Shtylyov
On 03/21/2016 11:41 PM, Uwe Kleine-König wrote: Commit 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") introduced a dependency on GPIOLIB that was not there before. This commit removes such dependency by checking the return code and comparing it

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Uwe Kleine-König
Hello Sergei, On Mon, Mar 21, 2016 at 11:15:13PM +0300, Sergei Shtylyov wrote: > On 03/16/2016 08:25 PM, Sebastian Frias wrote: > > >Commit 687908c2b649 ("net: phy: at803x: simplify using > >devm_gpiod_get_optional and its 4th argument") introduced a dependency > >on GPIOLIB that was not there

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Uwe Kleine-König
Hello Sergei, On Mon, Mar 21, 2016 at 11:15:13PM +0300, Sergei Shtylyov wrote: > On 03/16/2016 08:25 PM, Sebastian Frias wrote: > > >Commit 687908c2b649 ("net: phy: at803x: simplify using > >devm_gpiod_get_optional and its 4th argument") introduced a dependency > >on GPIOLIB that was not there

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sergei Shtylyov
Hello. On 03/16/2016 08:25 PM, Sebastian Frias wrote: Commit 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") introduced a dependency on GPIOLIB that was not there before. This commit removes such dependency by checking the return code and

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sergei Shtylyov
Hello. On 03/16/2016 08:25 PM, Sebastian Frias wrote: Commit 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") introduced a dependency on GPIOLIB that was not there before. This commit removes such dependency by checking the return code and

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Uwe Kleine-König
Hello Sebastian, On Mon, Mar 21, 2016 at 04:36:47PM +0100, Sebastian Frias wrote: > On 03/21/2016 02:54 PM, Uwe Kleine-König wrote: > >> - We'd also need to check that 'gpiod' is not NULL > > > > That is the optional part. If gpiod is NULL, you have one of the devices > > that don't need to

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Uwe Kleine-König
Hello Sebastian, On Mon, Mar 21, 2016 at 04:36:47PM +0100, Sebastian Frias wrote: > On 03/21/2016 02:54 PM, Uwe Kleine-König wrote: > >> - We'd also need to check that 'gpiod' is not NULL > > > > That is the optional part. If gpiod is NULL, you have one of the devices > > that don't need to

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sebastian Frias
Hi Uwe, On 03/21/2016 02:54 PM, Uwe Kleine-König wrote: >> >> Two things: >> - I suppose that in such hypothetical case the dependency on GPIOLIB >> would be required and thus be there > > I don't agree. There are bugs out there, and maybe the reason is as > simple as "the implementor of the

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sebastian Frias
Hi Uwe, On 03/21/2016 02:54 PM, Uwe Kleine-König wrote: >> >> Two things: >> - I suppose that in such hypothetical case the dependency on GPIOLIB >> would be required and thus be there > > I don't agree. There are bugs out there, and maybe the reason is as > simple as "the implementor of the

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Uwe Kleine-König
Hello Sebastian, On Mon, Mar 21, 2016 at 01:48:45PM +0100, Sebastian Frias wrote: > On 03/18/2016 08:12 PM, Uwe Kleine-König wrote: > > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > >> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: > >>> From a driver perspecitive, it would

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Uwe Kleine-König
Hello Sebastian, On Mon, Mar 21, 2016 at 01:48:45PM +0100, Sebastian Frias wrote: > On 03/18/2016 08:12 PM, Uwe Kleine-König wrote: > > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > >> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: > >>> From a driver perspecitive, it would

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sebastian Frias
Hi Uwe, On 03/18/2016 08:12 PM, Uwe Kleine-König wrote: > Hello Sebastian, > > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: >> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: >>> From a driver perspecitive, it would be nice if devm_gpiod_get_optional >>> returned NULL iff the

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-21 Thread Sebastian Frias
Hi Uwe, On 03/18/2016 08:12 PM, Uwe Kleine-König wrote: > Hello Sebastian, > > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: >> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: >>> From a driver perspecitive, it would be nice if devm_gpiod_get_optional >>> returned NULL iff the

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Uwe Kleine-König
Hello Sebastian, On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: > > From a driver perspecitive, it would be nice if devm_gpiod_get_optional > > returned NULL iff the respective gpio isn't specified even with > > GPIOLIB=n, but

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Uwe Kleine-König
Hello Sebastian, On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: > > From a driver perspecitive, it would be nice if devm_gpiod_get_optional > > returned NULL iff the respective gpio isn't specified even with > > GPIOLIB=n, but

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Uwe Kleine-König
Hello, On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: > On 18/03/2016 20:12, Uwe Kleine-König wrote: > > > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > > > >> What would you think of making at803x_link_change_notify() print a > >> message every time it should do a

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Uwe Kleine-König
Hello, On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: > On 18/03/2016 20:12, Uwe Kleine-König wrote: > > > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > > > >> What would you think of making at803x_link_change_notify() print a > >> message every time it should do a

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Mason
On 18/03/2016 21:11, Uwe Kleine-König wrote: > Hello, > > On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: > >> On 18/03/2016 20:12, Uwe Kleine-König wrote: >> >>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: >>> What would you think of making

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Mason
On 18/03/2016 21:11, Uwe Kleine-König wrote: > Hello, > > On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: > >> On 18/03/2016 20:12, Uwe Kleine-König wrote: >> >>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: >>> What would you think of making

[PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Sebastian Frias
Commit 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") introduced a dependency on GPIOLIB that was not there before. This commit removes such dependency by checking the return code and comparing it against ENOSYS which is returned when GPIOLIB is not

[PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Sebastian Frias
Commit 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument") introduced a dependency on GPIOLIB that was not there before. This commit removes such dependency by checking the return code and comparing it against ENOSYS which is returned when GPIOLIB is not

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Uwe Kleine-König
[expand cc a bit more] Hello, On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote: > Commit 687908c2b649 ("net: phy: at803x: simplify using > devm_gpiod_get_optional and its 4th argument") introduced a dependency > on GPIOLIB that was not there before. > > This commit removes such

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Uwe Kleine-König
[expand cc a bit more] Hello, On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote: > Commit 687908c2b649 ("net: phy: at803x: simplify using > devm_gpiod_get_optional and its 4th argument") introduced a dependency > on GPIOLIB that was not there before. > > This commit removes such

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Måns Rullgård
Mason writes: > On 18/03/2016 21:11, Uwe Kleine-König wrote: > >> Hello, >> >> On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: >> >>> On 18/03/2016 20:12, Uwe Kleine-König wrote: >>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > What

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Måns Rullgård
Mason writes: > On 18/03/2016 21:11, Uwe Kleine-König wrote: > >> Hello, >> >> On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: >> >>> On 18/03/2016 20:12, Uwe Kleine-König wrote: >>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > What would you think of

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Sebastian Frias
Hi Uwe, Daniel, On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: > [expand cc a bit more] > > Hello, > > On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote: >> Commit 687908c2b649 ("net: phy: at803x: simplify using >> devm_gpiod_get_optional and its 4th argument") introduced a

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Sebastian Frias
Hi Uwe, Daniel, On 03/18/2016 01:54 PM, Uwe Kleine-König wrote: > [expand cc a bit more] > > Hello, > > On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote: >> Commit 687908c2b649 ("net: phy: at803x: simplify using >> devm_gpiod_get_optional and its 4th argument") introduced a

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Mason
[ CCing a few devs who might be interested ] On 16/03/2016 18:25, Sebastian Frias wrote: > Commit 687908c2b649 ("net: phy: at803x: simplify using > devm_gpiod_get_optional and its 4th argument") introduced a dependency > on GPIOLIB that was not there before. > > This commit removes such

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Mason
[ CCing a few devs who might be interested ] On 16/03/2016 18:25, Sebastian Frias wrote: > Commit 687908c2b649 ("net: phy: at803x: simplify using > devm_gpiod_get_optional and its 4th argument") introduced a dependency > on GPIOLIB that was not there before. > > This commit removes such

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Mason
On 18/03/2016 20:12, Uwe Kleine-König wrote: > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > >> What would you think of making at803x_link_change_notify() print a >> message every time it should do a reset but does not has a way to do it? > > Then this question is obsolete

Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

2016-03-19 Thread Mason
On 18/03/2016 20:12, Uwe Kleine-König wrote: > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > >> What would you think of making at803x_link_change_notify() print a >> message every time it should do a reset but does not has a way to do it? > > Then this question is obsolete