[PATCH] spi: tegra: don't treat NULL clk as an error

[PATCH] spi: tegra: don't treat NULL clk as an error

[Jamie Iles]

Some platforms have been known to return NULL from clk_get() if they
support only a single struct clk.  Whilst tegra doesn't do this, make
the drivers consistent with others.

Cc: Erik Gilling <konkers@android.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---

 drivers/spi/spi_tegra.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
index bb7df02..891e590 100644
--- a/drivers/spi/spi_tegra.c
+++ b/drivers/spi/spi_tegra.c
@@ -513,7 +513,7 @@ static int __init spi_tegra_probe(struct platform_device *pdev)
 	}
 
 	tspi->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR_OR_NULL(tspi->clk)) {
+	if (IS_ERR(tspi->clk)) {
 		dev_err(&pdev->dev, "can not get clock\n");
 		ret = PTR_ERR(tspi->clk);
 		goto err2;
-- 
1.7.3.4

[PATCH] spi: tegra: don't treat NULL clk as an error

[Grant Likely]

On Mon, Jan 10, 2011 at 4:05 AM, Jamie Iles <jamie@jamieiles.com> wrote:
> Some platforms have been known to return NULL from clk_get() if they
> support only a single struct clk. ?Whilst tegra doesn't do this, make
> the drivers consistent with others.
>
> Cc: Erik Gilling <konkers@android.com>
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>

Hi James,

If NULL does get returned, say due to a future change to the clock
code, then this change causes the driver to oops.  I'm not going to
apply this patch.

g.

> ---
>
> ?drivers/spi/spi_tegra.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
> index bb7df02..891e590 100644
> --- a/drivers/spi/spi_tegra.c
> +++ b/drivers/spi/spi_tegra.c
> @@ -513,7 +513,7 @@ static int __init spi_tegra_probe(struct platform_device *pdev)
> ? ? ? ?}
>
> ? ? ? ?tspi->clk = clk_get(&pdev->dev, NULL);
> - ? ? ? if (IS_ERR_OR_NULL(tspi->clk)) {
> + ? ? ? if (IS_ERR(tspi->clk)) {
> ? ? ? ? ? ? ? ?dev_err(&pdev->dev, "can not get clock\n");
> ? ? ? ? ? ? ? ?ret = PTR_ERR(tspi->clk);
> ? ? ? ? ? ? ? ?goto err2;
> --
> 1.7.3.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



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

[PATCH] spi: tegra: don't treat NULL clk as an error

[Russell King - ARM Linux]

On Mon, Jan 10, 2011 at 01:58:12PM -0700, Grant Likely wrote:
> On Mon, Jan 10, 2011 at 4:05 AM, Jamie Iles <jamie@jamieiles.com> wrote:
> > Some platforms have been known to return NULL from clk_get() if they
> > support only a single struct clk. ?Whilst tegra doesn't do this, make
> > the drivers consistent with others.
> >
> > Cc: Erik Gilling <konkers@android.com>
> > Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> 
> Hi James,
> 
> If NULL does get returned, say due to a future change to the clock
> code, then this change causes the driver to oops.  I'm not going to
> apply this patch.

Please apply it - the clock API just defines struct clk as a cookie
where errors are IS_ERR/PTR_ERR.  Other values must be considered by
drivers as perfectly valid, including NULL.

[PATCH] spi: tegra: don't treat NULL clk as an error

[Russell King - ARM Linux]

On Mon, Jan 10, 2011 at 10:18:02PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 10, 2011 at 01:58:12PM -0700, Grant Likely wrote:
> > On Mon, Jan 10, 2011 at 4:05 AM, Jamie Iles <jamie@jamieiles.com> wrote:
> > > Some platforms have been known to return NULL from clk_get() if they
> > > support only a single struct clk. ?Whilst tegra doesn't do this, make
> > > the drivers consistent with others.
> > >
> > > Cc: Erik Gilling <konkers@android.com>
> > > Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> > 
> > Hi James,
> > 
> > If NULL does get returned, say due to a future change to the clock
> > code, then this change causes the driver to oops.  I'm not going to
> > apply this patch.
> 
> Please apply it - the clock API just defines struct clk as a cookie
> where errors are IS_ERR/PTR_ERR.  Other values must be considered by
> drivers as perfectly valid, including NULL.

Also note that drivers have no business ever dereferencing struct clks.
So if they oops, that's their own fault for dereferencing something
that they shouldn't.

[PATCH] spi: tegra: don't treat NULL clk as an error

[Grant Likely]

On Mon, Jan 10, 2011 at 3:25 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 10, 2011 at 10:18:02PM +0000, Russell King - ARM Linux wrote:
>> On Mon, Jan 10, 2011 at 01:58:12PM -0700, Grant Likely wrote:
>> > On Mon, Jan 10, 2011 at 4:05 AM, Jamie Iles <jamie@jamieiles.com> wrote:
>> > > Some platforms have been known to return NULL from clk_get() if they
>> > > support only a single struct clk. ?Whilst tegra doesn't do this, make
>> > > the drivers consistent with others.
>> > >
>> > > Cc: Erik Gilling <konkers@android.com>
>> > > Signed-off-by: Jamie Iles <jamie@jamieiles.com>
>> >
>> > Hi James,
>> >
>> > If NULL does get returned, say due to a future change to the clock
>> > code, then this change causes the driver to oops. ?I'm not going to
>> > apply this patch.
>>
>> Please apply it - the clock API just defines struct clk as a cookie
>> where errors are IS_ERR/PTR_ERR. ?Other values must be considered by
>> drivers as perfectly valid, including NULL.
>
> Also note that drivers have no business ever dereferencing struct clks.
> So if they oops, that's their own fault for dereferencing something
> that they shouldn't.

I was actually looking at the implementation of clk_enable() which is
called by the driver and doesn't do any NULL checking.  But I suppose
that it then becomes the clock-code's own fault if it returns NULL and
then oopses on a NULL being passed to it.  Okay, I'll apply it.

g.

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

[PATCH] spi: tegra: don't treat NULL clk as an error

[Russell King - ARM Linux]

On Mon, Jan 10, 2011 at 04:24:17PM -0700, Grant Likely wrote:
> I was actually looking at the implementation of clk_enable() which is
> called by the driver and doesn't do any NULL checking.  But I suppose
> that it then becomes the clock-code's own fault if it returns NULL and
> then oopses on a NULL being passed to it.  Okay, I'll apply it.

Yes absolutely.  The clk API must eat whatever cookies it produces,
even if they contain dead flies rather than currants. ;)

[PATCH] spi: tegra: don't treat NULL clk as an error

[Grant Likely]

On Mon, Jan 10, 2011 at 11:30:33PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 10, 2011 at 04:24:17PM -0700, Grant Likely wrote:
> > I was actually looking at the implementation of clk_enable() which is
> > called by the driver and doesn't do any NULL checking.  But I suppose
> > that it then becomes the clock-code's own fault if it returns NULL and
> > then oopses on a NULL being passed to it.  Okay, I'll apply it.
> 
> Yes absolutely.  The clk API must eat whatever cookies it produces,
> even if they contain dead flies rather than currants. ;)

nice analogy.  :-)

g.