drivers/w1/: why is dscore.c not ds9490r.c ?

drivers/w1/: why is dscore.c not ds9490r.c ?

[Adrian Bunk]

Hi Evgeniy,

drivers/w1/Makefile in recent 2.6 kernels contains:
  obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
  ds9490r-objs    := dscore.o

Is there a reason, why dscore.c isn't simply named ds9490r.c ?

TIA
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

On Mon, 2004-11-22 at 01:02, Adrian Bunk wrote:
> Hi Evgeniy,

Hello, Adrian.

> drivers/w1/Makefile in recent 2.6 kernels contains:
>   obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
>   ds9490r-objs    := dscore.o
> 
> Is there a reason, why dscore.c isn't simply named ds9490r.c ?

dscore.c is a core function set to work with ds2490 chip.
ds9490* is built on top of it.
Any vendor can create it's own w1 bus master using this chip, 
not ds9490.


> TIA
> Adrian
-- 
	Evgeniy Polyakov

Crash is better than data corruption. -- Art Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Adrian Bunk]

On Mon, Nov 22, 2004 at 10:31:12AM +0300, Evgeniy Polyakov wrote:
> On Mon, 2004-11-22 at 01:02, Adrian Bunk wrote:
> > Hi Evgeniy,
> 
> Hello, Adrian.

Hi Evgeniy,

> > drivers/w1/Makefile in recent 2.6 kernels contains:
> >   obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> >   ds9490r-objs    := dscore.o
> > 
> > Is there a reason, why dscore.c isn't simply named ds9490r.c ?
> 
> dscore.c is a core function set to work with ds2490 chip.
> ds9490* is built on top of it.
> Any vendor can create it's own w1 bus master using this chip, 
> not ds9490.

if it was built on top of it, I'd have expected ds9490r.o to contain 
additional object files.

How would a different w1 bus master chip look like in 
drivers/w1/Makefile?

> 	Evgeniy Polyakov

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On Mon, 2004-11-22 at 14:33 +0100, Adrian Bunk wrote:
> On Mon, Nov 22, 2004 at 10:31:12AM +0300, Evgeniy Polyakov wrote:
> > On Mon, 2004-11-22 at 01:02, Adrian Bunk wrote:
> > > Hi Evgeniy,
> > 
> > Hello, Adrian.
> 
> Hi Evgeniy,
> 
> > > drivers/w1/Makefile in recent 2.6 kernels contains:
> > >   obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> > >   ds9490r-objs    := dscore.o
> > > 
> > > Is there a reason, why dscore.c isn't simply named ds9490r.c ?
> > 
> > dscore.c is a core function set to work with ds2490 chip.
> > ds9490* is built on top of it.
> > Any vendor can create it's own w1 bus master using this chip, 
> > not ds9490.
> 
> if it was built on top of it, I'd have expected ds9490r.o to contain 
> additional object files.

DS9490 does not have anything except this chip and simple 64bit memory
chip,
so it is not needed to have any additional code.

> How would a different w1 bus master chip look like in 
> drivers/w1/Makefile?

obj-m: proprietary_module.o
proprietary_module-objs: dscore.o proprietary_module_init.o

Actually it will live outside the kernel tree, but will require ds2490
driver.
It could be called ds2490.c but I think dscore is better name.

> > 	Evgeniy Polyakov
> 
> cu
> Adrian
> 
-- 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Adrian Bunk]

On Mon, Nov 22, 2004 at 07:25:45PM +0300, Evgeniy Polyakov wrote:
> On Mon, 2004-11-22 at 14:33 +0100, Adrian Bunk wrote:
> > On Mon, Nov 22, 2004 at 10:31:12AM +0300, Evgeniy Polyakov wrote:
> > > On Mon, 2004-11-22 at 01:02, Adrian Bunk wrote:
> > > > Hi Evgeniy,
> > > 
> > > Hello, Adrian.
> > 
> > Hi Evgeniy,
> > 
> > > > drivers/w1/Makefile in recent 2.6 kernels contains:
> > > >   obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> > > >   ds9490r-objs    := dscore.o
> > > > 
> > > > Is there a reason, why dscore.c isn't simply named ds9490r.c ?
> > > 
> > > dscore.c is a core function set to work with ds2490 chip.
> > > ds9490* is built on top of it.
> > > Any vendor can create it's own w1 bus master using this chip, 
> > > not ds9490.
> > 
> > if it was built on top of it, I'd have expected ds9490r.o to contain 
> > additional object files.
> 
> DS9490 does not have anything except this chip and simple 64bit memory
> chip,
> so it is not needed to have any additional code.
> 
> > How would a different w1 bus master chip look like in 
> > drivers/w1/Makefile?
> 
> obj-m: proprietary_module.o
> proprietary_module-objs: dscore.o proprietary_module_init.o
> 
> Actually it will live outside the kernel tree, but will require ds2490
> driver.
> It could be called ds2490.c but I think dscore is better name.

Why are you talking about proprietary modules living outside the kernel 
tree?

The only interesting case is the one of modules shipped with the kernel.
And for them, this will break at link time if two such modules are 
included statically into the kernel.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

On Mon, 2004-11-22 at 17:51 +0100, Adrian Bunk wrote:
> On Mon, Nov 22, 2004 at 07:25:45PM +0300, Evgeniy Polyakov wrote:
> > On Mon, 2004-11-22 at 14:33 +0100, Adrian Bunk wrote:
> > > On Mon, Nov 22, 2004 at 10:31:12AM +0300, Evgeniy Polyakov wrote:
> > > > On Mon, 2004-11-22 at 01:02, Adrian Bunk wrote:
> > > > > Hi Evgeniy,
> > > > 
> > > > Hello, Adrian.
> > > 
> > > Hi Evgeniy,
> > > 
> > > > > drivers/w1/Makefile in recent 2.6 kernels contains:
> > > > >   obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> > > > >   ds9490r-objs    := dscore.o
> > > > > 
> > > > > Is there a reason, why dscore.c isn't simply named ds9490r.c ?
> > > > 
> > > > dscore.c is a core function set to work with ds2490 chip.
> > > > ds9490* is built on top of it.
> > > > Any vendor can create it's own w1 bus master using this chip, 
> > > > not ds9490.
> > > 
> > > if it was built on top of it, I'd have expected ds9490r.o to contain 
> > > additional object files.
> > 
> > DS9490 does not have anything except this chip and simple 64bit memory
> > chip,
> > so it is not needed to have any additional code.
> > 
> > > How would a different w1 bus master chip look like in 
> > > drivers/w1/Makefile?
> > 
> > obj-m: proprietary_module.o
> > proprietary_module-objs: dscore.o proprietary_module_init.o
> > 
> > Actually it will live outside the kernel tree, but will require ds2490
> > driver.
> > It could be called ds2490.c but I think dscore is better name.
> 
> Why are you talking about proprietary modules living outside the kernel 
> tree?
> 
> The only interesting case is the one of modules shipped with the kernel.
> And for them, this will break at link time if two such modules are 
> included statically into the kernel.

If we _currently_ do not have any open hw/module that depends on ds2490
core then it does not
mean that tomorrow noone will add it.

> cu
> Adrian
> 
-- 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Adrian Bunk]

On Mon, Nov 22, 2004 at 08:05:09PM +0300, Evgeniy Polyakov wrote:
> On Mon, 2004-11-22 at 17:51 +0100, Adrian Bunk wrote:
> > On Mon, Nov 22, 2004 at 07:25:45PM +0300, Evgeniy Polyakov wrote:
> > > 
> > > > How would a different w1 bus master chip look like in 
> > > > drivers/w1/Makefile?
> > > 
> > > obj-m: proprietary_module.o
> > > proprietary_module-objs: dscore.o proprietary_module_init.o
> > > 
> > > Actually it will live outside the kernel tree, but will require ds2490
> > > driver.
> > > It could be called ds2490.c but I think dscore is better name.
> > 
> > Why are you talking about proprietary modules living outside the kernel 
> > tree?
> > 
> > The only interesting case is the one of modules shipped with the kernel.
> > And for them, this will break at link time if two such modules are 
> > included statically into the kernel.
> 
> If we _currently_ do not have any open hw/module that depends on ds2490
> core then it does not
> mean that tomorrow noone will add it.

Once again:
  _this will break at link time if two such modules are included 
   statically into the kernel_

obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
ds9490r-objs    := dscore.o

obj-$(CONFIG_W1_FOO)         += foo.o 
foo-objs    := dscore.o


This will break with CONFIG_W1_DS9490=y and CONFIG_W1_FOO=y.


That drivers/w1/ contains many EXPORT_SYMBOL's with no in-kernel users 
is a different issue I might send a separate patch for (that besides 
proprietary modules there might come some day open source drivers using 
them is not a reason).


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

On Mon, 2004-11-22 at 18:19 +0100, Adrian Bunk wrote:
> On Mon, Nov 22, 2004 at 08:05:09PM +0300, Evgeniy Polyakov wrote:
> > On Mon, 2004-11-22 at 17:51 +0100, Adrian Bunk wrote:
> > > On Mon, Nov 22, 2004 at 07:25:45PM +0300, Evgeniy Polyakov wrote:
> > > > 
> > > > > How would a different w1 bus master chip look like in 
> > > > > drivers/w1/Makefile?
> > > > 
> > > > obj-m: proprietary_module.o
> > > > proprietary_module-objs: dscore.o proprietary_module_init.o
> > > > 
> > > > Actually it will live outside the kernel tree, but will require ds2490
> > > > driver.
> > > > It could be called ds2490.c but I think dscore is better name.
> > > 
> > > Why are you talking about proprietary modules living outside the kernel 
> > > tree?
> > > 
> > > The only interesting case is the one of modules shipped with the kernel.
> > > And for them, this will break at link time if two such modules are 
> > > included statically into the kernel.
> > 
> > If we _currently_ do not have any open hw/module that depends on ds2490
> > core then it does not
> > mean that tomorrow noone will add it.
> 
> Once again:
>   _this will break at link time if two such modules are included 
>    statically into the kernel_
> 
> obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> ds9490r-objs    := dscore.o
> 
> obj-$(CONFIG_W1_FOO)         += foo.o 
> foo-objs    := dscore.o
> 

that should be follwing:

Kconfig:
foo depends on ds9490r

obj-$() += foo.o
foo-objs := foo_1.o foo_2.o

It just happened that ds9490r does not need any other parts but
dscore.o.
That is why ds9490r.o have only dscore.o in it's dependency.

> This will break with CONFIG_W1_DS9490=y and CONFIG_W1_FOO=y.
> 
> 
> That drivers/w1/ contains many EXPORT_SYMBOL's with no in-kernel users 
> is a different issue I might send a separate patch for (that besides 
> proprietary modules there might come some day open source drivers using 
> them is not a reason).

Why remove existing non disturbing set of exported functions?
Are they violate some unknown issues?

> 
> cu
> Adrian
> 
-- 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Adrian Bunk]

On Mon, Nov 22, 2004 at 08:37:00PM +0300, Evgeniy Polyakov wrote:
> On Mon, 2004-11-22 at 18:19 +0100, Adrian Bunk wrote:
> > On Mon, Nov 22, 2004 at 08:05:09PM +0300, Evgeniy Polyakov wrote:
> > > On Mon, 2004-11-22 at 17:51 +0100, Adrian Bunk wrote:
> > > > On Mon, Nov 22, 2004 at 07:25:45PM +0300, Evgeniy Polyakov wrote:
> > > > > 
> > > > > > How would a different w1 bus master chip look like in 
> > > > > > drivers/w1/Makefile?
> > > > > 
> > > > > obj-m: proprietary_module.o
> > > > > proprietary_module-objs: dscore.o proprietary_module_init.o
> > > > > 
> > > > > Actually it will live outside the kernel tree, but will require ds2490
> > > > > driver.
> > > > > It could be called ds2490.c but I think dscore is better name.
> > > > 
> > > > Why are you talking about proprietary modules living outside the kernel 
> > > > tree?
> > > > 
> > > > The only interesting case is the one of modules shipped with the kernel.
> > > > And for them, this will break at link time if two such modules are 
> > > > included statically into the kernel.
> > > 
> > > If we _currently_ do not have any open hw/module that depends on ds2490
> > > core then it does not
> > > mean that tomorrow noone will add it.
> > 
> > Once again:
> >   _this will break at link time if two such modules are included 
> >    statically into the kernel_
> > 
> > obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> > ds9490r-objs    := dscore.o
> > 
> > obj-$(CONFIG_W1_FOO)         += foo.o 
> > foo-objs    := dscore.o
> > 
> 
> that should be follwing:
> 
> Kconfig:
> foo depends on ds9490r
> 
> obj-$() += foo.o
> foo-objs := foo_1.o foo_2.o
> 
> It just happened that ds9490r does not need any other parts but
> dscore.o.
> That is why ds9490r.o have only dscore.o in it's dependency.

If foo_1 or foo_2 is dscore, you get exactly the compile breakage I 
described.

> > This will break with CONFIG_W1_DS9490=y and CONFIG_W1_FOO=y.
> > 
> > 
> > That drivers/w1/ contains many EXPORT_SYMBOL's with no in-kernel users 
> > is a different issue I might send a separate patch for (that besides 
> > proprietary modules there might come some day open source drivers using 
> > them is not a reason).
> 
> Why remove existing non disturbing set of exported functions?
> Are they violate some unknown issues?

If an export is currently unused, there's no need to export it.

If an export is only used for proprietary modules, that's a reason for 
an immediate removal of this export.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

On Tue, 2004-11-23 at 01:20 +0100, Adrian Bunk wrote:
> On Mon, Nov 22, 2004 at 08:37:00PM +0300, Evgeniy Polyakov wrote:
> > On Mon, 2004-11-22 at 18:19 +0100, Adrian Bunk wrote:
> > > On Mon, Nov 22, 2004 at 08:05:09PM +0300, Evgeniy Polyakov wrote:
> > > > On Mon, 2004-11-22 at 17:51 +0100, Adrian Bunk wrote:
> > > > > On Mon, Nov 22, 2004 at 07:25:45PM +0300, Evgeniy Polyakov wrote:
> > > > > > 
> > > > > > > How would a different w1 bus master chip look like in 
> > > > > > > drivers/w1/Makefile?
> > > > > > 
> > > > > > obj-m: proprietary_module.o
> > > > > > proprietary_module-objs: dscore.o proprietary_module_init.o
> > > > > > 
> > > > > > Actually it will live outside the kernel tree, but will require ds2490
> > > > > > driver.
> > > > > > It could be called ds2490.c but I think dscore is better name.
> > > > > 
> > > > > Why are you talking about proprietary modules living outside the kernel 
> > > > > tree?
> > > > > 
> > > > > The only interesting case is the one of modules shipped with the kernel.
> > > > > And for them, this will break at link time if two such modules are 
> > > > > included statically into the kernel.
> > > > 
> > > > If we _currently_ do not have any open hw/module that depends on ds2490
> > > > core then it does not
> > > > mean that tomorrow noone will add it.
> > > 
> > > Once again:
> > >   _this will break at link time if two such modules are included 
> > >    statically into the kernel_
> > > 
> > > obj-$(CONFIG_W1_DS9490)         += ds9490r.o 
> > > ds9490r-objs    := dscore.o
> > > 
> > > obj-$(CONFIG_W1_FOO)         += foo.o 
> > > foo-objs    := dscore.o
> > > 
> > 
> > that should be follwing:
> > 
> > Kconfig:
> > foo depends on ds9490r
> > 
> > obj-$() += foo.o
> > foo-objs := foo_1.o foo_2.o
> > 
> > It just happened that ds9490r does not need any other parts but
> > dscore.o.
> > That is why ds9490r.o have only dscore.o in it's dependency.
> 
> If foo_1 or foo_2 is dscore, you get exactly the compile breakage I 
> described.

foo_1 and foo_2 will not be dscore, since foo depends on ds9490 and thus
dscore must be already built.
It looks like you were confused by dscore vs. ds9490 names. Probably it
was not a good idea to
call it in a such way, but it was done and it works.

> > > This will break with CONFIG_W1_DS9490=y and CONFIG_W1_FOO=y.
> > > 
> > > 
> > > That drivers/w1/ contains many EXPORT_SYMBOL's with no in-kernel users 
> > > is a different issue I might send a separate patch for (that besides 
> > > proprietary modules there might come some day open source drivers using 
> > > them is not a reason).
> > 
> > Why remove existing non disturbing set of exported functions?
> > Are they violate some unknown issues?
> 
> If an export is currently unused, there's no need to export it.
> 
> If an export is only used for proprietary modules, that's a reason for 
> an immediate removal of this export.

Sigh. It can be used by anyone who want to use it.
I will take more carefull look at it later, thank you.

> cu
> Adrian
> 
-- 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: drivers/w1/: why is dscore.c not ds9490r.c ?

[Adrian Bunk]

On Tue, Nov 23, 2004 at 01:34:12PM +0300, Evgeniy Polyakov wrote:
> On Tue, 2004-11-23 at 01:20 +0100, Adrian Bunk wrote:
> > On Mon, Nov 22, 2004 at 08:37:00PM +0300, Evgeniy Polyakov wrote:
> > > 
> > > that should be follwing:
> > > 
> > > Kconfig:
> > > foo depends on ds9490r
> > > 
> > > obj-$() += foo.o
> > > foo-objs := foo_1.o foo_2.o
> > > 
> > > It just happened that ds9490r does not need any other parts but
> > > dscore.o.
> > > That is why ds9490r.o have only dscore.o in it's dependency.
> > 
> > If foo_1 or foo_2 is dscore, you get exactly the compile breakage I 
> > described.
> 
> foo_1 and foo_2 will not be dscore, since foo depends on ds9490 and thus
> dscore must be already built.
> It looks like you were confused by dscore vs. ds9490 names. Probably it
> was not a good idea to
> call it in a such way, but it was done and it works.

Is there a reason against simply renaming dscore.c to ds9490r.c ?

> > > > That drivers/w1/ contains many EXPORT_SYMBOL's with no in-kernel users 
> > > > is a different issue I might send a separate patch for (that besides 
> > > > proprietary modules there might come some day open source drivers using 
> > > > them is not a reason).
> > > 
> > > Why remove existing non disturbing set of exported functions?
> > > Are they violate some unknown issues?
> > 
> > If an export is currently unused, there's no need to export it.
> > 
> > If an export is only used for proprietary modules, that's a reason for 
> > an immediate removal of this export.
> 
> Sigh. It can be used by anyone who want to use it.

That's not the way it's handled in the Linux kernel.

Proprietary modules are allowed, but support code only required by them 
but not used by free software has to be removed.

> I will take more carefull look at it later, thank you.

Thanks.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

[2.6 patch] drivers/w1/: possible cleanups

[Adrian Bunk]

The patch below includes the following possible cleanups:
- make needlessly global code static
- remove unused code

Please review and comment which parts are correct and which conflict 
with pending changes that will add in-kernel users for the functions in 
question.


diffstat output:
 drivers/w1/dscore.c    |  103 +++++++++++------------------------------
 drivers/w1/dscore.h    |    7 --
 drivers/w1/w1.c        |   25 ++++-----
 drivers/w1/w1.h        |    1 
 drivers/w1/w1_family.c |   16 +-----
 drivers/w1/w1_family.h |    2 
 drivers/w1/w1_int.c    |    7 --
 drivers/w1/w1_int.h    |    2 
 drivers/w1/w1_io.c     |   13 ++---
 drivers/w1/w1_io.h     |    3 -
 10 files changed, 51 insertions(+), 128 deletions(-)


Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.h.old	2004-11-29 02:07:27.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.h	2004-11-29 02:12:44.000000000 +0100
@@ -156,14 +156,7 @@
 int ds_read_bit(struct ds_device *, u8 *);
 int ds_write_byte(struct ds_device *, u8);
 int ds_write_bit(struct ds_device *, u8);
-int ds_start_pulse(struct ds_device *, int);
-int ds_set_speed(struct ds_device *, int);
 int ds_reset(struct ds_device *, struct ds_status *);
-int ds_detect(struct ds_device *, struct ds_status *);
-int ds_stop_pulse(struct ds_device *, int);
-int ds_send_data(struct ds_device *, unsigned char *, int);
-int ds_recv_data(struct ds_device *, unsigned char *, int);
-int ds_recv_status(struct ds_device *, struct ds_status *);
 struct ds_device * ds_get_device(void);
 void ds_put_device(struct ds_device *);
 int ds_write_block(struct ds_device *, u8 *, int);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.c.old	2004-11-29 02:07:43.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.c	2004-11-29 02:24:28.000000000 +0100
@@ -32,22 +32,19 @@
 };
 MODULE_DEVICE_TABLE(usb, ds_id_table);
 
-int ds_probe(struct usb_interface *, const struct usb_device_id *);
-void ds_disconnect(struct usb_interface *);
+static int ds_probe(struct usb_interface *, const struct usb_device_id *);
+static void ds_disconnect(struct usb_interface *);
 
 int ds_touch_bit(struct ds_device *, u8, u8 *);
 int ds_read_byte(struct ds_device *, u8 *);
 int ds_read_bit(struct ds_device *, u8 *);
 int ds_write_byte(struct ds_device *, u8);
 int ds_write_bit(struct ds_device *, u8);
-int ds_start_pulse(struct ds_device *, int);
-int ds_set_speed(struct ds_device *, int);
+static int ds_start_pulse(struct ds_device *, int);
 int ds_reset(struct ds_device *, struct ds_status *);
-int ds_detect(struct ds_device *, struct ds_status *);
-int ds_stop_pulse(struct ds_device *, int);
-int ds_send_data(struct ds_device *, unsigned char *, int);
-int ds_recv_data(struct ds_device *, unsigned char *, int);
-int ds_recv_status(struct ds_device *, struct ds_status *);
+static int ds_send_data(struct ds_device *, unsigned char *, int);
+static int ds_recv_data(struct ds_device *, unsigned char *, int);
+static int ds_recv_status(struct ds_device *, struct ds_status *);
 struct ds_device * ds_get_device(void);
 void ds_put_device(struct ds_device *);
 
@@ -129,7 +126,7 @@
 	printk("%45s: %8x\n", str, buf[off]);
 }
 
-int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st, unsigned char *buf, int size)
+static int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st, unsigned char *buf, int size)
 {
 	int count, err;
 		
@@ -148,7 +145,7 @@
 	return count;
 }
 
-int ds_recv_status(struct ds_device *dev, struct ds_status *st)
+static int ds_recv_status(struct ds_device *dev, struct ds_status *st)
 {
 	unsigned char buf[64];
 	int count, err = 0, i;
@@ -206,7 +203,7 @@
 	return err;
 }
 
-int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size)
+static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size)
 {
 	int count, err;
 	struct ds_status st;
@@ -234,7 +231,7 @@
 	return count;
 }
 
-int ds_send_data(struct ds_device *dev, unsigned char *buf, int len)
+static int ds_send_data(struct ds_device *dev, unsigned char *buf, int len)
 {
 	int count, err;
 	
@@ -248,7 +245,8 @@
 	return err;
 }
 
-int ds_stop_pulse(struct ds_device *dev, int limit)
+#if 0
+static int ds_stop_pulse(struct ds_device *dev, int limit)
 {
 	struct ds_status st;
 	int count = 0, err = 0;
@@ -274,33 +272,9 @@
 
 	return err;
 }
+#endif
 
-int ds_detect(struct ds_device *dev, struct ds_status *st)
-{
-	int err;
-	
-	err = ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
-	if (err)
-		return err;
-
-	err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM, 0);
-	if (err)
-		return err;
-	
-	err = ds_send_control(dev, COMM_SET_DURATION | COMM_IM | COMM_TYPE, 0x40);
-	if (err)
-		return err;
-	
-	err = ds_send_control_mode(dev, MOD_PULSE_EN, PULSE_PROG);
-	if (err)
-		return err;
-
-	err = ds_recv_status(dev, st);
-
-	return err;
-}
-
-int ds_wait_status(struct ds_device *dev, struct ds_status *st)
+static int ds_wait_status(struct ds_device *dev, struct ds_status *st)
 {
 	u8 buf[0x20];
 	int err, count = 0;
@@ -348,26 +322,7 @@
 	return 0;
 }
 
-int ds_set_speed(struct ds_device *dev, int speed)
-{
-	int err;
-	
-	if (speed != SPEED_NORMAL && speed != SPEED_FLEXIBLE && speed != SPEED_OVERDRIVE)
-		return -EINVAL;
-
-	if (speed != SPEED_OVERDRIVE)
-		speed = SPEED_FLEXIBLE;
-
-	speed &= 0xff;
-	
-	err = ds_send_control_mode(dev, MOD_1WIRE_SPEED, speed);
-	if (err)
-		return err;
-
-	return err;
-}
-
-int ds_start_pulse(struct ds_device *dev, int delay)
+static int ds_start_pulse(struct ds_device *dev, int delay)
 {
 	int err;
 	u8 del = 1 + (u8)(delay >> 4);
@@ -555,7 +510,8 @@
 	return !(err == len);
 }
 
-int ds_search(struct ds_device *dev, u64 init, u64 *buf, u8 id_number, int conditional_search)
+#if 0
+static int ds_search(struct ds_device *dev, u64 init, u64 *buf, u8 id_number, int conditional_search)
 {
 	int err;
 	u16 value, index;
@@ -583,8 +539,10 @@
 
 	return err/8;
 }
+#endif
 
-int ds_match_access(struct ds_device *dev, u64 init)
+#if 0
+static int ds_match_access(struct ds_device *dev, u64 init)
 {
 	int err;
 	struct ds_status st;
@@ -603,8 +561,10 @@
 
 	return 0;
 }
+#endif
 
-int ds_set_path(struct ds_device *dev, u64 init)
+#if 0
+static int ds_set_path(struct ds_device *dev, u64 init)
 {
 	int err;
 	struct ds_status st;
@@ -627,8 +587,9 @@
 
 	return 0;
 }
+#endif
 
-int ds_probe(struct usb_interface *intf, const struct usb_device_id *udev_id)
+static int ds_probe(struct usb_interface *intf, const struct usb_device_id *udev_id)
 {
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct usb_endpoint_descriptor *endpoint;
@@ -723,7 +684,7 @@
 	return 0;
 }
 
-void ds_disconnect(struct usb_interface *intf)
+static void ds_disconnect(struct usb_interface *intf)
 {
 	struct ds_device *dev;
 	
@@ -743,7 +704,7 @@
 	ds_dev = NULL;
 }
 
-int ds_init(void)
+static int ds_init(void)
 {
 	int err;
 
@@ -756,7 +717,7 @@
 	return 0;
 }
 
-void ds_fini(void)
+static void ds_fini(void)
 {
 	usb_deregister(&ds_driver);
 }
@@ -774,15 +735,7 @@
 EXPORT_SYMBOL(ds_write_byte);
 EXPORT_SYMBOL(ds_write_bit);
 EXPORT_SYMBOL(ds_write_block);
-EXPORT_SYMBOL(ds_start_pulse);
-EXPORT_SYMBOL(ds_set_speed);
 EXPORT_SYMBOL(ds_reset);
-EXPORT_SYMBOL(ds_detect);
-EXPORT_SYMBOL(ds_stop_pulse);
-EXPORT_SYMBOL(ds_send_data);
-EXPORT_SYMBOL(ds_recv_data);
-EXPORT_SYMBOL(ds_recv_status);
-EXPORT_SYMBOL(ds_search);
 EXPORT_SYMBOL(ds_get_device);
 EXPORT_SYMBOL(ds_put_device);
 
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.h.old	2004-11-29 02:19:06.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.h	2004-11-29 02:19:32.000000000 +0100
@@ -27,8 +27,6 @@
 
 #include "w1.h"
 
-struct w1_master * w1_alloc_dev(u32, int, int, struct device_driver *, struct device *);
-void w1_free_dev(struct w1_master *dev);
 int w1_add_master_device(struct w1_bus_master *);
 void w1_remove_master_device(struct w1_bus_master *);
 void __w1_remove_master_device(struct w1_master *);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.c.old	2004-11-29 02:13:29.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.c	2004-11-29 02:19:40.000000000 +0100
@@ -30,7 +30,6 @@
 static u32 w1_ids = 1;
 
 extern struct device_driver w1_driver;
-extern struct bus_type w1_bus_type;
 extern struct device w1_device;
 extern int w1_max_slave_count;
 extern int w1_max_slave_ttl;
@@ -39,7 +38,7 @@
 
 extern int w1_process(void *);
 
-struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
+static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
 	      struct device_driver *driver, struct device *device)
 {
 	struct w1_master *dev;
@@ -109,7 +108,7 @@
 	return dev;
 }
 
-void w1_free_dev(struct w1_master *dev)
+static void w1_free_dev(struct w1_master *dev)
 {
 	device_unregister(&dev->dev);
 	if (dev->nls->sk_socket)
@@ -220,8 +219,6 @@
 	__w1_remove_master_device(dev);
 }
 
-EXPORT_SYMBOL(w1_alloc_dev);
-EXPORT_SYMBOL(w1_free_dev);
 EXPORT_SYMBOL(w1_add_master_device);
 EXPORT_SYMBOL(w1_remove_master_device);
 EXPORT_SYMBOL(__w1_remove_master_device);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1.h.old	2004-11-29 02:14:36.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1.h	2004-11-29 02:14:40.000000000 +0100
@@ -126,7 +126,6 @@
 };
 
 int w1_create_master_attributes(struct w1_master *);
-void w1_destroy_master_attributes(struct w1_master *);
 
 #endif /* __KERNEL__ */
 
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1.c.old	2004-11-29 02:13:43.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1.c	2004-11-29 02:16:47.000000000 +0100
@@ -101,7 +101,7 @@
 	return sprintf(buf, "No family registered.\n");
 }
 
-struct bus_type w1_bus_type = {
+static struct bus_type w1_bus_type = {
 	.name = "w1",
 	.match = w1_master_match,
 };
@@ -139,7 +139,7 @@
 	.show = &w1_default_read_name,
 };
 
-ssize_t w1_master_attribute_show_name(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_name(struct device *dev, char *buf)
 {
 	struct w1_master *md = container_of (dev, struct w1_master, dev);
 	ssize_t count;
@@ -154,7 +154,7 @@
 	return count;
 }
 
-ssize_t w1_master_attribute_show_pointer(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_pointer(struct device *dev, char *buf)
 {
 	struct w1_master *md = container_of(dev, struct w1_master, dev);
 	ssize_t count;
@@ -168,14 +168,14 @@
 	return count;
 }
 
-ssize_t w1_master_attribute_show_timeout(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_timeout(struct device *dev, char *buf)
 {
 	ssize_t count;
 	count = sprintf(buf, "%d\n", w1_timeout);
 	return count;
 }
 
-ssize_t w1_master_attribute_show_max_slave_count(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_max_slave_count(struct device *dev, char *buf)
 {
 	struct w1_master *md = container_of(dev, struct w1_master, dev);
 	ssize_t count;
@@ -189,7 +189,7 @@
 	return count;
 }
 
-ssize_t w1_master_attribute_show_attempts(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_attempts(struct device *dev, char *buf)
 {
 	struct w1_master *md = container_of(dev, struct w1_master, dev);
 	ssize_t count;
@@ -203,7 +203,7 @@
 	return count;
 }
 
-ssize_t w1_master_attribute_show_slave_count(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_slave_count(struct device *dev, char *buf)
 {
 	struct w1_master *md = container_of(dev, struct w1_master, dev);
 	ssize_t count;
@@ -217,7 +217,7 @@
 	return count;
 }
 
-ssize_t w1_master_attribute_show_slaves(struct device *dev, char *buf)
+static ssize_t w1_master_attribute_show_slaves(struct device *dev, char *buf)
 
 {
 	struct w1_master *md = container_of(dev, struct w1_master, dev);
@@ -600,7 +600,7 @@
 	return 0;
 }
 
-void w1_destroy_master_attributes(struct w1_master *dev)
+static void w1_destroy_master_attributes(struct w1_master *dev)
 {
 	device_remove_file(&dev->dev, &w1_master_attribute_slaves);
 	device_remove_file(&dev->dev, &w1_master_attribute_slave_count);
@@ -612,7 +612,7 @@
 }
 
 
-int w1_control(void *data)
+static int w1_control(void *data)
 {
 	struct w1_slave *sl;
 	struct w1_master *dev;
@@ -749,7 +749,7 @@
 	return 0;
 }
 
-int w1_init(void)
+static int w1_init(void)
 {
 	int retval;
 
@@ -789,7 +789,7 @@
 	return retval;
 }
 
-void w1_fini(void)
+static void w1_fini(void)
 {
 	struct w1_master *dev;
 	struct list_head *ent, *n;
@@ -811,4 +811,3 @@
 module_exit(w1_fini);
 
 EXPORT_SYMBOL(w1_create_master_attributes);
-EXPORT_SYMBOL(w1_destroy_master_attributes);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.h.old	2004-11-29 02:17:17.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.h	2004-11-29 02:18:19.000000000 +0100
@@ -54,10 +54,8 @@
 
 extern spinlock_t w1_flock;
 
-void w1_family_get(struct w1_family *);
 void w1_family_put(struct w1_family *);
 void __w1_family_get(struct w1_family *);
-void __w1_family_put(struct w1_family *);
 struct w1_family * w1_family_registered(u8);
 void w1_unregister_family(struct w1_family *);
 int w1_register_family(struct w1_family *);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.c.old	2004-11-29 02:17:29.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.c	2004-11-29 02:18:38.000000000 +0100
@@ -115,25 +115,17 @@
 	return (ret) ? f : NULL;
 }
 
-void w1_family_put(struct w1_family *f)
-{
-	spin_lock(&w1_flock);
-	__w1_family_put(f);
-	spin_unlock(&w1_flock);
-}
-
-void __w1_family_put(struct w1_family *f)
+static void __w1_family_put(struct w1_family *f)
 {
 	if (atomic_dec_and_test(&f->refcnt))
 		f->need_exit = 1;
 }
 
-void w1_family_get(struct w1_family *f)
+void w1_family_put(struct w1_family *f)
 {
 	spin_lock(&w1_flock);
-	__w1_family_get(f);
+	__w1_family_put(f);
 	spin_unlock(&w1_flock);
-
 }
 
 void __w1_family_get(struct w1_family *f)
@@ -141,10 +133,8 @@
 	atomic_inc(&f->refcnt);
 }
 
-EXPORT_SYMBOL(w1_family_get);
 EXPORT_SYMBOL(w1_family_put);
 EXPORT_SYMBOL(__w1_family_get);
-EXPORT_SYMBOL(__w1_family_put);
 EXPORT_SYMBOL(w1_family_registered);
 EXPORT_SYMBOL(w1_unregister_family);
 EXPORT_SYMBOL(w1_register_family);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.h.old	2004-11-29 02:19:54.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.h	2004-11-29 02:21:08.000000000 +0100
@@ -24,12 +24,9 @@
 
 #include "w1.h"
 
-void w1_delay(unsigned long);
 u8 w1_touch_bit(struct w1_master *, int);
 void w1_write_bit(struct w1_master *, int);
 void w1_write_8(struct w1_master *, u8);
-u8 w1_read_bit(struct w1_master *);
-u8 w1_read_8(struct w1_master *);
 int w1_reset_bus(struct w1_master *);
 u8 w1_calc_crc8(u8 *, int);
 void w1_write_block(struct w1_master *, u8 *, int);
--- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.c.old	2004-11-29 02:20:11.000000000 +0100
+++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.c	2004-11-29 02:21:39.000000000 +0100
@@ -28,7 +28,9 @@
 #include "w1_log.h"
 #include "w1_io.h"
 
-int w1_delay_parm = 1;
+static u8 w1_read_bit(struct w1_master *dev);
+
+static int w1_delay_parm = 1;
 module_param_named(delay_coef, w1_delay_parm, int, 0);
 
 static u8 w1_crc8_table[] = {
@@ -50,7 +52,7 @@
 	116, 42, 200, 150, 21, 75, 169, 247, 182, 232, 10, 84, 215, 137, 107, 53
 };
 
-void w1_delay(unsigned long tm)
+static void w1_delay(unsigned long tm)
 {
 	udelay(tm * w1_delay_parm);
 }
@@ -89,7 +91,7 @@
 			w1_write_bit(dev, (byte >> i) & 0x1);
 }
 
-u8 w1_read_bit(struct w1_master *dev)
+static u8 w1_read_bit(struct w1_master *dev)
 {
 	int result;
 
@@ -104,7 +106,7 @@
 	return result & 0x1;
 }
 
-u8 w1_read_8(struct w1_master * dev)
+static u8 w1_read_8(struct w1_master * dev)
 {
 	int i;
 	u8 res = 0;
@@ -176,10 +178,7 @@
 
 EXPORT_SYMBOL(w1_write_bit);
 EXPORT_SYMBOL(w1_write_8);
-EXPORT_SYMBOL(w1_read_bit);
-EXPORT_SYMBOL(w1_read_8);
 EXPORT_SYMBOL(w1_reset_bus);
 EXPORT_SYMBOL(w1_calc_crc8);
-EXPORT_SYMBOL(w1_delay);
 EXPORT_SYMBOL(w1_read_block);
 EXPORT_SYMBOL(w1_write_block);

Re: [2.6 patch] drivers/w1/: possible cleanups

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 14721 bytes --]

On Mon, 2004-11-29 at 02:52 +0100, Adrian Bunk wrote:
> The patch below includes the following possible cleanups:
> - make needlessly global code static
> - remove unused code
> 
> Please review and comment which parts are correct and which conflict 
> with pending changes that will add in-kernel users for the functions in 
> question.

Thank you, but I will not apply your patch as is, since many parts of it
are generic
and need to be used for any ds2490 based devices.

Comments below.

> diffstat output:
>  drivers/w1/dscore.c    |  103 +++++++++++------------------------------
>  drivers/w1/dscore.h    |    7 --
>  drivers/w1/w1.c        |   25 ++++-----
>  drivers/w1/w1.h        |    1 
>  drivers/w1/w1_family.c |   16 +-----
>  drivers/w1/w1_family.h |    2 
>  drivers/w1/w1_int.c    |    7 --
>  drivers/w1/w1_int.h    |    2 
>  drivers/w1/w1_io.c     |   13 ++---
>  drivers/w1/w1_io.h     |    3 -
>  10 files changed, 51 insertions(+), 128 deletions(-)
> 
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.h.old	2004-11-29 02:07:27.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.h	2004-11-29 02:12:44.000000000 +0100
> @@ -156,14 +156,7 @@
>  int ds_read_bit(struct ds_device *, u8 *);
>  int ds_write_byte(struct ds_device *, u8);
>  int ds_write_bit(struct ds_device *, u8);
> -int ds_start_pulse(struct ds_device *, int);
> -int ds_set_speed(struct ds_device *, int);
>  int ds_reset(struct ds_device *, struct ds_status *);
> -int ds_detect(struct ds_device *, struct ds_status *);
> -int ds_stop_pulse(struct ds_device *, int);
> -int ds_send_data(struct ds_device *, unsigned char *, int);
> -int ds_recv_data(struct ds_device *, unsigned char *, int);
> -int ds_recv_status(struct ds_device *, struct ds_status *);

1. ds_start_pulse() and ds_set_speed() are needed for eeprom programming
and setting device speed.
2. ds_{send,recv}_data can be used to receive bulks of data.
3. ds_send_status() is used to obtain status.

You may say that they are used only in ds9490 driver, but not -
they are generic functions to control any ds2490 based device.

Here is example: if I want to enable network only for netlink
communication
I will get even some qdisk functions exported.
Any above and some below functions _can_ be used to control ds2490 based
hardware.
DS9490R is _just_one_ of such devices. ds9490r is built on top of
(actually it is only)
dscore.o.
dscore.o - this is _generic_ driver for DS2490 based hardware. 
Some of it requires above function, others not.
When(if) kernel connector patch will be applied, many of this functions
can be accessed by it, 
but nevertheless they are needed for some hardware, and thus can be
accessed from it's drivers.

>  struct ds_device * ds_get_device(void);
>  void ds_put_device(struct ds_device *);
>  int ds_write_block(struct ds_device *, u8 *, int);
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.c.old	2004-11-29 02:07:43.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/dscore.c	2004-11-29 02:24:28.000000000 +0100
> @@ -32,22 +32,19 @@
>  };
>  MODULE_DEVICE_TABLE(usb, ds_id_table);
>  
> -int ds_probe(struct usb_interface *, const struct usb_device_id *);
> -void ds_disconnect(struct usb_interface *);
> +static int ds_probe(struct usb_interface *, const struct usb_device_id *);
> +static void ds_disconnect(struct usb_interface *);

I agree here - this functions are Linux USB callbacks and can be static.

<unacceptible static vs. nostaticchanges are skipped>

> -int ds_init(void)
> +static int ds_init(void)
>  {
>  	int err;
>  
> @@ -756,7 +717,7 @@
>  	return 0;
>  }
>  
> -void ds_fini(void)
> +static void ds_fini(void)
>  {
>  	usb_deregister(&ds_driver);
>  }

Sure, it is right changes.

> @@ -774,15 +735,7 @@
>  EXPORT_SYMBOL(ds_write_byte);
>  EXPORT_SYMBOL(ds_write_bit);
>  EXPORT_SYMBOL(ds_write_block);
> -EXPORT_SYMBOL(ds_start_pulse);
> -EXPORT_SYMBOL(ds_set_speed);
>  EXPORT_SYMBOL(ds_reset);
> -EXPORT_SYMBOL(ds_detect);
> -EXPORT_SYMBOL(ds_stop_pulse);
> -EXPORT_SYMBOL(ds_send_data);
> -EXPORT_SYMBOL(ds_recv_data);
> -EXPORT_SYMBOL(ds_recv_status);
> -EXPORT_SYMBOL(ds_search);
>  EXPORT_SYMBOL(ds_get_device);
>  EXPORT_SYMBOL(ds_put_device);
>  
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.h.old	2004-11-29 02:19:06.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.h	2004-11-29 02:19:32.000000000 +0100
> @@ -27,8 +27,6 @@
>  
>  #include "w1.h"
>  
> -struct w1_master * w1_alloc_dev(u32, int, int, struct device_driver *, struct device *);
> -void w1_free_dev(struct w1_master *dev);

This can be static too.

>  int w1_add_master_device(struct w1_bus_master *);
>  void w1_remove_master_device(struct w1_bus_master *);
>  void __w1_remove_master_device(struct w1_master *);
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.c.old	2004-11-29 02:13:29.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_int.c	2004-11-29 02:19:40.000000000 +0100
> @@ -30,7 +30,6 @@
>  static u32 w1_ids = 1;
>  
>  extern struct device_driver w1_driver;
> -extern struct bus_type w1_bus_type;

Ok.

>  extern struct device w1_device;
>  extern int w1_max_slave_count;
>  extern int w1_max_slave_ttl;
> @@ -39,7 +38,7 @@
>  
>  extern int w1_process(void *);
>  
> -struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
> +static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
>  	      struct device_driver *driver, struct device *device)
>  {
>  	struct w1_master *dev;
> @@ -109,7 +108,7 @@
>  	return dev;
>  }
>  
> -void w1_free_dev(struct w1_master *dev)
> +static void w1_free_dev(struct w1_master *dev)
>  {
>  	device_unregister(&dev->dev);
>  	if (dev->nls->sk_socket)
> @@ -220,8 +219,6 @@
>  	__w1_remove_master_device(dev);
>  }
>  
> -EXPORT_SYMBOL(w1_alloc_dev);
> -EXPORT_SYMBOL(w1_free_dev);

As I say this change is right.

>  EXPORT_SYMBOL(w1_add_master_device);
>  EXPORT_SYMBOL(w1_remove_master_device);
>  EXPORT_SYMBOL(__w1_remove_master_device);
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1.h.old	2004-11-29 02:14:36.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1.h	2004-11-29 02:14:40.000000000 +0100
> @@ -126,7 +126,6 @@
>  };
>  
>  int w1_create_master_attributes(struct w1_master *);
> -void w1_destroy_master_attributes(struct w1_master *);

I've exported it for completeness - it was used before in error exit
path.
I need to change it back - w1_create_master_attributes() must be called
before 
w1_process() thread creation - this will reduce error recovery path.

>  #endif /* __KERNEL__ */
>  
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1.c.old	2004-11-29 02:13:43.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1.c	2004-11-29 02:16:47.000000000 +0100
> @@ -101,7 +101,7 @@
>  	return sprintf(buf, "No family registered.\n");
>  }
>  
> -struct bus_type w1_bus_type = {
> +static struct bus_type w1_bus_type = {
>  	.name = "w1",
>  	.match = w1_master_match,
>  };
> @@ -139,7 +139,7 @@
>  	.show = &w1_default_read_name,
>  };
>  
> -ssize_t w1_master_attribute_show_name(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_name(struct device *dev, char *buf)
>  {
>  	struct w1_master *md = container_of (dev, struct w1_master, dev);
>  	ssize_t count;
> @@ -154,7 +154,7 @@
>  	return count;
>  }
>  
> -ssize_t w1_master_attribute_show_pointer(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_pointer(struct device *dev, char *buf)
>  {
>  	struct w1_master *md = container_of(dev, struct w1_master, dev);
>  	ssize_t count;
> @@ -168,14 +168,14 @@
>  	return count;
>  }
>  
> -ssize_t w1_master_attribute_show_timeout(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_timeout(struct device *dev, char *buf)
>  {
>  	ssize_t count;
>  	count = sprintf(buf, "%d\n", w1_timeout);
>  	return count;
>  }
>  
> -ssize_t w1_master_attribute_show_max_slave_count(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_max_slave_count(struct device *dev, char *buf)
>  {
>  	struct w1_master *md = container_of(dev, struct w1_master, dev);
>  	ssize_t count;
> @@ -189,7 +189,7 @@
>  	return count;
>  }
>  
> -ssize_t w1_master_attribute_show_attempts(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_attempts(struct device *dev, char *buf)
>  {
>  	struct w1_master *md = container_of(dev, struct w1_master, dev);
>  	ssize_t count;
> @@ -203,7 +203,7 @@
>  	return count;
>  }
>  
> -ssize_t w1_master_attribute_show_slave_count(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_slave_count(struct device *dev, char *buf)
>  {
>  	struct w1_master *md = container_of(dev, struct w1_master, dev);
>  	ssize_t count;
> @@ -217,7 +217,7 @@
>  	return count;
>  }
>  
> -ssize_t w1_master_attribute_show_slaves(struct device *dev, char *buf)
> +static ssize_t w1_master_attribute_show_slaves(struct device *dev, char *buf)
>  
>  {
>  	struct w1_master *md = container_of(dev, struct w1_master, dev);
> @@ -600,7 +600,7 @@
>  	return 0;
>  }
>  
> -void w1_destroy_master_attributes(struct w1_master *dev)
> +static void w1_destroy_master_attributes(struct w1_master *dev)
>  {
>  	device_remove_file(&dev->dev, &w1_master_attribute_slaves);
>  	device_remove_file(&dev->dev, &w1_master_attribute_slave_count);
> @@ -612,7 +612,7 @@
>  }
>  
> 
> -int w1_control(void *data)
> +static int w1_control(void *data)
>  {
>  	struct w1_slave *sl;
>  	struct w1_master *dev;
> @@ -749,7 +749,7 @@
>  	return 0;
>  }
>  
> -int w1_init(void)
> +static int w1_init(void)
>  {
>  	int retval;
>  
> @@ -789,7 +789,7 @@
>  	return retval;
>  }
>  
> -void w1_fini(void)
> +static void w1_fini(void)
>  {
>  	struct w1_master *dev;
>  	struct list_head *ent, *n;
> @@ -811,4 +811,3 @@
>  module_exit(w1_fini);

I agree with above changes.

>  EXPORT_SYMBOL(w1_create_master_attributes);
> -EXPORT_SYMBOL(w1_destroy_master_attributes);
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.h.old	2004-11-29 02:17:17.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.h	2004-11-29 02:18:19.000000000 +0100
> @@ -54,10 +54,8 @@
>  
>  extern spinlock_t w1_flock;
>  
> -void w1_family_get(struct w1_family *);
>  void w1_family_put(struct w1_family *);
>  void __w1_family_get(struct w1_family *);
> -void __w1_family_put(struct w1_family *);
>  struct w1_family * w1_family_registered(u8);
>  void w1_unregister_family(struct w1_family *);
>  int w1_register_family(struct w1_family *);
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.c.old	2004-11-29 02:17:29.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_family.c	2004-11-29 02:18:38.000000000 +0100
> @@ -115,25 +115,17 @@
>  	return (ret) ? f : NULL;
>  }
>  
> -void w1_family_put(struct w1_family *f)
> -{
> -	spin_lock(&w1_flock);
> -	__w1_family_put(f);
> -	spin_unlock(&w1_flock);
> -}
> -
> -void __w1_family_put(struct w1_family *f)
> +static void __w1_family_put(struct w1_family *f)
>  {
>  	if (atomic_dec_and_test(&f->refcnt))
>  		f->need_exit = 1;
>  }
>  
> -void w1_family_get(struct w1_family *f)
> +void w1_family_put(struct w1_family *f)
>  {
>  	spin_lock(&w1_flock);
> -	__w1_family_get(f);
> +	__w1_family_put(f);
>  	spin_unlock(&w1_flock);
> -
>  }
>  
>  void __w1_family_get(struct w1_family *f)
> @@ -141,10 +133,8 @@
>  	atomic_inc(&f->refcnt);
>  }
>  
> -EXPORT_SYMBOL(w1_family_get);
>  EXPORT_SYMBOL(w1_family_put);
>  EXPORT_SYMBOL(__w1_family_get);
> -EXPORT_SYMBOL(__w1_family_put);
>  EXPORT_SYMBOL(w1_family_registered);
>  EXPORT_SYMBOL(w1_unregister_family);
>  EXPORT_SYMBOL(w1_register_family);

You break good writting style.

> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.h.old	2004-11-29 02:19:54.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.h	2004-11-29 02:21:08.000000000 +0100
> @@ -24,12 +24,9 @@
>  
>  #include "w1.h"
>  
> -void w1_delay(unsigned long);
>  u8 w1_touch_bit(struct w1_master *, int);
>  void w1_write_bit(struct w1_master *, int);
>  void w1_write_8(struct w1_master *, u8);
> -u8 w1_read_bit(struct w1_master *);
> -u8 w1_read_8(struct w1_master *);
>  int w1_reset_bus(struct w1_master *);
>  u8 w1_calc_crc8(u8 *, int);
>  void w1_write_block(struct w1_master *, u8 *, int);
> --- linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.c.old	2004-11-29 02:20:11.000000000 +0100
> +++ linux-2.6.10-rc2-mm3-full/drivers/w1/w1_io.c	2004-11-29 02:21:39.000000000 +0100
> @@ -28,7 +28,9 @@
>  #include "w1_log.h"
>  #include "w1_io.h"
>  
> -int w1_delay_parm = 1;
> +static u8 w1_read_bit(struct w1_master *dev);
> +
> +static int w1_delay_parm = 1;
>  module_param_named(delay_coef, w1_delay_parm, int, 0);

This can not be static - embedded setups without modules support with
unusual 
HZ value need this value to create hacks over udelay() to control
protocol timslices.
I have discussion with some arm people about it - this will be changed
when kernel 
connector will be in mainline.

>  static u8 w1_crc8_table[] = {
> @@ -50,7 +52,7 @@
>  	116, 42, 200, 150, 21, 75, 169, 247, 182, 232, 10, 84, 215, 137, 107, 53
>  };
>  
> -void w1_delay(unsigned long tm)
> +static void w1_delay(unsigned long tm)
>  {
>  	udelay(tm * w1_delay_parm);
>  }
> @@ -89,7 +91,7 @@
>  			w1_write_bit(dev, (byte >> i) & 0x1);
>  }
>  
> -u8 w1_read_bit(struct w1_master *dev)
> +static u8 w1_read_bit(struct w1_master *dev)
>  {
>  	int result;
>  
> @@ -104,7 +106,7 @@
>  	return result & 0x1;
>  }
>  
> -u8 w1_read_8(struct w1_master * dev)
> +static u8 w1_read_8(struct w1_master * dev)
>  {
>  	int i;
>  	u8 res = 0;
> @@ -176,10 +178,7 @@
>  
>  EXPORT_SYMBOL(w1_write_bit);
>  EXPORT_SYMBOL(w1_write_8);
> -EXPORT_SYMBOL(w1_read_bit);
> -EXPORT_SYMBOL(w1_read_8);
>  EXPORT_SYMBOL(w1_reset_bus);
>  EXPORT_SYMBOL(w1_calc_crc8);
> -EXPORT_SYMBOL(w1_delay);
>  EXPORT_SYMBOL(w1_read_block);
>  EXPORT_SYMBOL(w1_write_block);

You break it again.


Resume:
1. Lets wait untill current pending patches will be in mainline - it
includes your previous changes.
2. I will push upstream some of the above changes after 1.

I greatly appreciate your help, thank you Adrian.

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]