Add support for configuring pin for w1-gpio kernel module

[dpenezic]

Patch implement runtime configuration option for setting GPIO pin like 1-Wire master device.

When use module you may use it on follow way :
modprobe w1-gpio (standard hard coded pin 4 will be used)
modprobe w1-gpio gpiopin=18 (pin 18 will be used for 1-Wire devices. Pin number need to be real pin number not pin position on GPIO interface)

More about issue may be found on http://www.raspberrypi.org/phpBB3/viewtopic.php?f=37&t=40044

[popcornmix]

Note there are 3 unwanted commits here. Can you rebase and force push?

[dpenezic]

Hi,
i am very new with submitting patches ... May you give me some pointer/URL to read how to do that ? I was searching , but didnt found any info about submitting patch to RPi .
Regards,
Dubravko Penezic

[popcornmix]

I'd suggest something like:

git checkout w1-patch
git fetch origin
git reset --hard origin/HEAD
git cherry-pick e09690d
git push -f

[dpenezic]

Hi,
i did steps you suggest ... may i use same system ( i think i learn what variable parts of this command is )for further patches ?

Regards,
Dubravko Penezic

[popcornmix]

Yes, your git rebase worked correctly. That scheme should work fine on other PRs.
In a PR, you can always modify your local branch how you like and then force push, and the PR will update.
There's a couple of indentation issues. Can you fix that locally and do:

git commit --amend
git push -f

and this PR should update.

[dpenezic]

Hi,
i think i fix indentation issues. Please check.

Regards,
Dubravko Penezic

P.S.: Just for my future info (or maybe some one found helpfull :) ) i will rewrite your last block of command to

git add w1-gpio.c
git commit --amend
git push -f

[dpenezic]

Open

[popcornmix]

I believe GPIO 0 is a valid option, but your code won't allow it to be selected.
Would it make sense to initialise "gpiopin = -1" and then treat "gpiopin >= 0" as the valid condition?

[dpenezic]

Yes, you are right 0 is valid value .. i made a change and resubmit patch ...

[popcornmix]

It feels to me this is platform specific code, rather than part of the w1 driver which already has a API for setting gpio pin number.
Wouldn't it make sense for this to be set in bcm2708.c just before:
platform_device_register(&w1_device);

@FrankBuss any comments?

[dpenezic]

Hi,
maybe i have stupid question, but if i like to use more then one pin for 1-wire communication ? Isnt be possible to call 2 or more times same module with different parameters ?

Also I am not expert for kernels, and know just a little bit, but isnt all value for bcm2708 preconfigured on kernel compilation time ?

Regards,
Dubravko Penezic

[FrankBuss]

I don't have much time and didn't test the patch, but some remarks:

• pdata->pin is overwritten with the parameter and then used for gpio_request, but looks like gpio_free uses w1_gpio_orig
• I think S_IWUSR won't work (changing the parameter while the module is loaded)
• maybe popcornmix is right, would be better to set it in bcm2708.c, but I don't know if it is possible to pass a module parameter to it

@dpenezic I don't know if it is possible to load it twice. At least there is just one "/sys/bus/w1/devices/". And I guess you are right and the bcm2708 platform data is set on kernel compilation time, so maybe not possible to change it with a module parameter. Which would be a reason to patch the w1-gpio.c. But if the platform data can't be changed anyway, a more clean solution would be to add a function like this:

static u8 get_pin(struct w1_gpio_platform_data *pdata)
{
        return gpiopin < 0 ? pdata->pin : gpiopin;
}

(or use a macro) instead of changing the immutable platform data and then using it for any places which references pdata->pin.

[dpenezic]

Hi,
@FrankBuss I know that patch work :) ... because at list 4 people is used, including myself :) (check link to RPi forum on beginning of this thread)...

I am not C/C++ programmer (understand some principles, may do some small changes), so i will need check all your suggestive connected to code and try to implement it..

@ALL
I like that system is configurable (defaults are O.K.) but from time to time you need to do something different (many reasons may occur in real live).

If you need to use kernel module to achieve something then right place to configure work of that module is to add configuration parameter(s) to it.

@popcornmix You said that new version of w1_gpio.c has implemented new API, where I can found proper git source ? Maybe good decision is to change source complete.

[popcornmix]

@dpenezic
No, I didn't mention a new w1_gpio.c. I just said that it already contains an API for setting the GPIO pin when instantiating the driver.

My suggestion id to move the module parameter to bcm2708.c. Call it (for example) w1-gpiopin.
Assign this parameter to w1_gpio_pdata.pin before doing:

platform_device_register(&w1_device);

You can then add:
bcm2708.w1-gpiopin=18

to cmdline.txt to set a specific pin. Unfortunately this scheme requires a reboot when switching GPIO pins, but for most users that would be a rare event.

It has the big advantage that it only changes our platform specific code, and not the generic upstream w1_gpoi.c.

[dpenezic]

@popcornmix ... sorry for late answer ... your explanation have a sense ... reboot isnt problem at all ... that will fix most case ... do you have maybe any suggestion where to read some basic info how to do that in bcm2708.c ... or i may look into file and try something similar which is implement yet ?

Regards,
Dubravko Penezic

[popcornmix]

This is my suggestion:
http://pastebin.com/dHBUHFE4
I don't have a 1-wire device, but I believe if you add:

bcm2708.w1_gpio_pin=8

to /boot/cmdline.txt and reboot, then the 1-wire module will use GPIO 8.

Can you test this works for you?

[dpenezic]

@popcornmix i will try to test and send you information back soon i get one RPi for test.

Regards,
Dubravko Penezic

[dpenezic]

@popcornmix just to inform you that patch for bcm2708.c which you suggest work like we expect :) ... i made git branch with change.

[popcornmix]

My patch is in latest update. Please rpi-update and test.

[dpenezic]

Hi, i have information from two user that new implemented patch work correctly, i will compile my kernel tomorrow, and give final result.
Regards,
Dubravko Penezic

[dpenezic]

Hi, just to confirm everything work perfect with new Raspbian 2013-12-20 .

Regards,
Dubravko Penezic