linux-kernel - Re: [RFC][PATCH 2/4] RTC: SWARM I2C board initialization

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: <20080509092835.3bbf0f55@hyperion.delvare>
Date:	Fri, 9 May 2008 09:28:35 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	"Maciej W. Rozycki" <macro@...ux-mips.org>
Cc:	Ralf Baechle <ralf@...ux-mips.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	rtc-linux@...glegroups.com, i2c@...sensors.org,
	linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org,
	David Brownell <david-b@...bell.net>
Subject: Re: [RFC][PATCH 2/4] RTC: SWARM I2C board initialization

Hi Maciej,

On Fri, 9 May 2008 00:10:47 +0100 (BST), Maciej W. Rozycki wrote:
> > I'm not going to tell how bad I think the GNU coding standards are, the
> > point here is that we don't follow them at all, so whatever they say is
> > totally irrelevant. Read Documentation/CodingStyle, it describes what
> 
>  Oh come on -- that's just common sense.  If something is good, there is
> no point in discarding it without thinking, just because it is a part of a
> bigger entity that we consider bad.  I consider it good not because it is
> a part of the GNU standard, but because I have concluded that it is and it
> is pure coincidence ;-) I have taken it from the said standard.

Let me just quote you:

"This is mostly habitual -- this is what the GNU Coding Standard specifies
for comments and which is enforced for GNU software which I have dealt a
lot with."

You didn't say it was common sense. You did say that it was what the
GNU Coding Standard specified, and as a consequence, what you were used
to. So please keep your "oh come on" for yourself, you pointed the
discussion in this direction yourself.

>                                                                   But as I
> said, this is a minor nit here and I can resist from adding extraneous
> spaces in pieces of code you are interested in as long as I am able to
> track which ones they actually are.

What matters is not "the pieces of code I am interested in", but the
pieces of code _you_ are the master of, or not. As explained somewhere
else in this thread, you are free to use whatever style you like (as
long as it complies with Documentation/CodingStyle, that is) in new
code you write and in code you maintain. For all the rest, you should
stick to the surrounding style. This is common sense, as you'd say.

> (...)
>  Well, arch/mips/sibyte/swarm/ is included for all the three above as well 
> as a couple of other I may not necessarily be sure what they are even.  So 
> this should be of no concern.
> 
>  BTW, do you mean i2c_add_numbered_adapter() will fail if no devices have
> been declared to exist on the given bus with i2c_register_board_info()?  
> That sounds strange...

i2c_add_numbered_adapter() _may_ fail if no I2C devices have been
declared _and_ other i2c adapters are registered using
i2c_add_adapter(). When you declare I2C devices, i2c-core reserves the
bus numbers in question for i2c_add_numbered_adapter() and they cannot
be used for i2c_add_adapter(). This is what guarantees that the calls
to i2c_add_numbered_adapter() (in i2c-sibyte for example) will succeed.
If no I2C devices are declared, bus numbers are not reserved, so if it
happens that another I2C bus driver registers itself before i2c-sibyte
does, when i2c-sibyte calls i2c_add_numbered_adapter(), the bus number
is already in use and the call fails.

I admit that this is unlikely to happen, but depending on what exact
hardware there is on the system and what drivers are built-in, it could
happen. I think it is a weakness of i2c-core, it should allow platform
code to reserve i2c bus numbers regardless of what devices are
registered. I seem to remember I said that when the code was added to
the kernel already. I guess we'll have to fix it the day it actually
breaks.

BTW, i2c-sibyte should be converted to a proper platform driver, so
that only platforms with such a device instantiate it.

>                         Note in all cases there are EEPROMs (onboard ones
> as well as optionally SPD ones) on both buses on Broadcom/SiByte boards
> and they are handled by a legacy client driver.  The Broadcom SOC is
> actually capable to bootstrap from one of these EEPROMs (rather than form
> the usual system parallel Flash ROM).

Which legacy driver, "eeprom"? You should probably look into David
Brownell's at24c driver:
http://lists.lm-sensors.org/pipermail/i2c/2008-April/003307.html
If it gets enough attention and testing, it could go upstream quickly.

-- 
Jean Delvare
--
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/