linux-kernel - Re: [PATCH 08/20] Staging: ipack: Switch to 8MHz operation before reading ID.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120911113919.GA7174@endeavour.taprogge.org>
Date:	Tue, 11 Sep 2012 13:39:19 +0200
From:	Jens Taprogge <jens.taprogge@...rogge.org>
To:	Samuel Iglesias Gonsálvez 
	<siglesias@...lia.com>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
	industrypack-devel@...ts.sourceforge.net
Subject: Re: [PATCH 08/20] Staging: ipack: Switch to 8MHz operation before
 reading ID.

On Tue, Sep 11, 2012 at 01:31:06PM +0200, Samuel Iglesias Gonsálvez wrote:
> On Tue, 2012-09-11 at 11:48 +0300, Dan Carpenter wrote:
> > On Mon, Sep 10, 2012 at 10:51:46AM +0200, Samuel Iglesias Gonsálvez wrote:
> > > From: Jens Taprogge <jens.taprogge@...rogge.org>
> > > 
> > > Reading the ID space at 8 MHz is always supported.  Most carriers will
> > > boot up in 8MHz mode.  Still, play it safe and ensure we are operating at
> > > 8Mhz.
> > > 
> > > Signed-off-by: Jens Taprogge <jens.taprogge@...rogge.org>
> > > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@...lia.com>
> > > ---
> > >  drivers/staging/ipack/ipack.c |   11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/staging/ipack/ipack.c b/drivers/staging/ipack/ipack.c
> > > index 4f3c945..95f56b8 100644
> > > --- a/drivers/staging/ipack/ipack.c
> > > +++ b/drivers/staging/ipack/ipack.c
> > > @@ -376,6 +376,9 @@ struct ipack_device *ipack_device_register(struct ipack_bus_device *bus,
> > >  	dev_set_name(&dev->dev,
> > >  		     "ipack-dev.%u.%u", dev->bus_nr, dev->slot);
> > >  
> > > +	if (bus->ops->set_clockrate(dev, 8))
> > > +		dev_warn(&dev->dev, "failed to switch to 8 MHz operation for reading of device ID.\n");
> > > +
> > >  	ret = ipack_device_read_id(dev);
> > >  	if (ret < 0) {
> > >  		dev_err(&dev->dev, "error reading device id section.\n");
> > > @@ -384,9 +387,11 @@ struct ipack_device *ipack_device_register(struct ipack_bus_device *bus,
> > >  	}
> > >  
> > >  	/* if the device supports 32 MHz operation, use it. */
> > > -	ret = bus->ops->set_clockrate(dev, dev->speed_32mhz ? 32 : 8);
> > > -	if (ret < 0)
> > > -		dev_err(&dev->dev, "failed to switch to 32 MHz operation.\n");
> > > +	if (dev->speed_32mhz) {
> > > +		ret = bus->ops->set_clockrate(dev, 32);
> > > +		if (ret < 0)
> > > +			dev_err(&dev->dev, "failed to switch to 32 MHz operation.\n");
> > > +	}
> > 
> > Ah.  Well done.  That's what I suggested earlier.  I think you
> > should get rid of the ->speed_8mhz all together.
> 
> I will do it in a follow-up patch. The rest of suggestions have been
> integrated in the patches. I will submit the patches very soon.

I have not seen any IPack V2 device yet.  But since the ID space
provides the 8MHz flag, it might be useful.  On the other hand, we can
still readd it when we see it.

When you remove the flag, you can perhaps leave a comment in the code so
that it is easy to re-enable.

Best Regards,
Jens
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/