Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1685581 > unrolled thread

Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Greg KH <gregkh@linuxfoundation.org> - 2017-07-12 09:50 +0200
    Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Greg KH <gregkh@linuxfoundation.org> - 2017-07-15 13:30 +0200
      Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-15 15:00 +0200
        Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Greg KH <gregkh@linuxfoundation.org> - 2017-07-15 15:10 +0200
          Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Greg KH <gregkh@linuxfoundation.org> - 2017-07-15 15:20 +0200
            Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Greg KH <gregkh@linuxfoundation.org> - 2017-07-15 15:50 +0200
              Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi Greg KH <gregkh@linuxfoundation.org> - 2017-07-16 08:50 +0200
                Re: [PATCH 1/1] drivers/staging/pi433: New driver Greg KH <gregkh@linuxfoundation.org> - 2017-07-16 12:20 +0200
                  Re: [PATCH 1/1] drivers/staging/pi433: New driver Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-16 16:00 +0200
                    Re: [PATCH 1/1] drivers/staging/pi433: New driver Greg KH <gregkh@linuxfoundation.org> - 2017-07-16 16:50 +0200

#1685581 — Re: Submit of a driver for Pi433 - a radio module for Raspberry Pi

On Tue, Jul 11, 2017 at 06:02:09PM +0200, Marcus Wolf wrote:
> Hi folks,
> 
> I developed a radio shield for the 433MHz ISM band [0] for the Raspberry Pi.
> This shield is called Pi433 [1]. It can be used to communicate between two
> Raspberries or to control third party equipment e. g. cheap radio sockets [2].
> The base of this radio shield is a radio module from HopeRf - a rfm69cw-433s2.
>  
> In the beginning, I conrolled the rf69 chip by direct access via SPI, but I
> figured out, that several tasks were hard to handle. Especially the following
> points were annoying:
> * Just one process could open the SPI interface so just one process was
>   able to access the module
> * A simultaneous send and receive was very hard to implement
> 
> Therefore I started to implement a driver.
> The drivers features:
> * simple access/nice abstraction of spi registers via open, close,
>   read, write and ioctl
> * multiple applications can open
> * tx requests are queued, so every app can send a tx request at any time
> * first app asking for rx will block other apps, also asking for rx until one
> rx
>   cycle is complete
> * rx cycle means listening on the air until something appears
> * if an app wants to tx during the driver is waiting in rx cycle, rx will be
>   shortly interrupted to do the tx task
> * for each tx task a seperate configuration of the rf chip (bitrate,
> modulation, ...)
>   can be chosen
> * wrtten in modular architecture, allowing to extend for other products with
>   rf69 chip (e.g. rfm69hcw, ...) or even to support other HopeRf chips with
> similar
>   interface (e. g. rfm12, rfm95, ...)
>  
> In principle the driver was intended to be used by my project (Smarthome-Pi),
> only.
> But while having all this nice features, I was asked, whether I could offer the
> driver to the community. I never did that before, so I started reading about
> submitting drivers and figured out, that there is a lot of work to do, to meet
> all
> concepts and ideas of complying to kernel code. I spend some days and modified
> a lot, but still, a lot of things aren't perfect.
> Already known tasks are:
> 
> * coding style does not fully comply with the kernel style guide.
> * still TODOs, annotated in the code
> * currently the code introduces new IOCTLs. I'm afraid this is a bad idea.
>   Replace this with another interface, hints are welcome!
>  
> I tested the patch on Raspbian with Kernel v4.12 and applied the patch on
> mainline Linux v4.12
>  
> Is it posibble to integrate into the staging area to ease further development?

Sure, I'm always glad to take new drivers, but can you submit it as just
a single patch, and keep it self-contained (i.e. nothing outside of
drivers/staging/YOUR_DRIVER/)?

thanks,

greg k-h

[toc] | [next] | [standalone]

#1687922

On Sat, Jul 15, 2017 at 01:15:43PM +0200, Marcus Wolf wrote:
> Hi Greg,
>  
> thanks for your reply :-)
>  
> Today I moved the documentation and header files to drivers/staging/pi433 and
> fromated it as a single patch.
>  
> Cheers,
>  
> Marcus

I still need it in a format I can apply it in (i.e. proper subject,
changelog text, signed-off-by, etc.)  Can you do that?

Also, for staging drivers, I need a TODO file, much like the existing
drivers/staging/*/TODO files saying what needs to be done with the code
in order to get it out of staging.

And finally, your patch seemed to have the whitespace corrupted and was
line-wrapped, making it impossible to apply even if I could have :(

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1687940

Hi!
 
It's me again. Seems like I also need help on sending the email :-/
 
I checked the whitespace/line wrap problem, but couldn't find any suspicious
lines.
 
What I did:
* Looked into my outbox - the copy of my mail to you seems to be okay...
* I sent the patch once again (just to me) - result: Seems to be fine, too.
 
Maybe you can bounce back my mail? Maybe I'll get an idea what went wrong, if I
see the smashed code...
 
Cheers,
 
Marcus
 

> Greg KH <gregkh@linuxfoundation.org> hat am 15. Juli 2017 um 13:24
> geschrieben:
>
>
> On Sat, Jul 15, 2017 at 01:15:43PM +0200, Marcus Wolf wrote:
> > Hi Greg,
> >
> > thanks for your reply :-)
> >
> > Today I moved the documentation and header files to drivers/staging/pi433
> > and
> > fromated it as a single patch.
> >
> > Cheers,
> >
> > Marcus
>
> I still need it in a format I can apply it in (i.e. proper subject,
> changelog text, signed-off-by, etc.) Can you do that?
>
> Also, for staging drivers, I need a TODO file, much like the existing
> drivers/staging/*/TODO files saying what needs to be done with the code
> in order to get it out of staging.
>
> And finally, your patch seemed to have the whitespace corrupted and was
> line-wrapped, making it impossible to apply even if I could have :(
>
> thanks,
>
> greg k-h
>

[toc] | [prev] | [next] | [standalone]

#1687943

On Sat, Jul 15, 2017 at 02:59:25PM +0200, Marcus Wolf wrote:
> Hi!
>  
> It's me again. Seems like I also need help on sending the email :-/
>  
> I checked the whitespace/line wrap problem, but couldn't find any suspicious
> lines.
>  
> What I did:
> * Looked into my outbox - the copy of my mail to you seems to be okay...
> * I sent the patch once again (just to me) - result: Seems to be fine, too.
>  
> Maybe you can bounce back my mail? Maybe I'll get an idea what went wrong, if I
> see the smashed code...

You sent the patch in html format, which caused all of the whitespace
corruption.  html will not work for kernel patches for obvious reasons
:)

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1687949

On Sat, Jul 15, 2017 at 03:10:48PM +0200, Marcus Wolf wrote:
> Hi!
>  
> Ok. Thanks for the info. Didn't observe that.
>  I'll give the old patch another try.
>  
> I will send a new patch as soon, as I know. how to produce the 
> poper format (with subjects, signing and so on).

Documentation/SubmittingPatches should have all of the information you
need for this type of thing.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1687951

On Sat, Jul 15, 2017 at 03:40:30PM +0200, Marcus Wolf wrote:
> Hi Greg,
>  
> I absolutly agree. My patch should meet the needs of an official patch.

Again, don't use html email :)

> But I don't know about the exact needs. Can you please help me? What is needed
> / missing in my patch and how do I produce it?
> Maybe you can pass me a link, where I can read, what to do?

The link I sent you, Documentation/SubmittingPatches in the kernel
source tree, should contain everything you need to know.  As a minimum
you need a good subject, a good changelog body text, and a
signed-off-by: line.  The file should tell you all about how to create
this.

If you have specific questions after reading that about these things,
please let us know.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1688133

On Sat, Jul 15, 2017 at 07:05:04PM +0200, Wolf Entwicklungen wrote:
> Am Sa, 15.07.2017, 15:47 schrieb Greg KH:
> > On Sat, Jul 15, 2017 at 03:40:30PM +0200, Marcus Wolf wrote:
> >> Hi Greg,
> >>
> 
> Hi Greg,
> 
> now I added a TODO file and did a manual patchwork with lines of the old patch
> (git format-patch master --stdout -p > pi433_patch) and the newer patch (git
> diff master > pi433_patch). Stil don't know how to retreive multiple commits
> in one single patch directly from git :-/
> I did my best to meet a lot of rules of the link, you send me.If
> there is stil something essential wrong or missing, please excuse and let me know.
> 
> Especially I was in doubt about the verbosity of the change log. If you'd like
> to see the list of features of the driver over there (please see my first mail),
> let me know and I'll add.


Can you resend it without all of this stuff here, a patch email should just
contain the patch itself, look at all of the examples on the mailing
lists for how they look.

> From: Marcus Wolf <linux@Wolf-Entwicklungen.de>
> Date: Tue,15 Jul 2017 17:52:06 +0100
> Subject: [PATCH 1/1] drivers/staging/pi433: New driver
> 
> Added a driver for the pi433 radio module
> (see https://www.pi433.de/en.html for details).
> Signed-off-by: Marcus Wolf <linux@Wolf-Entwicklungen.de>

Need a blank line before signed-off-by.

> Tested-by: Marcus Wolf <linux@Wolf-Entwicklungen.de> on Raspbian, running Kernel v4.12

No need for a tested-by from something that wrote, that is implied :)

Almost there...

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1688182 — Re: [PATCH 1/1] drivers/staging/pi433: New driver

On Sun, Jul 16, 2017 at 11:52:32AM +0200, Wolf Entwicklungen wrote:
> From: Marcus Wolf <linux@Wolf-Entwicklungen.de>
> Date: Tue,16 Jul 2017 11:52:06 +0100
> Subject: [PATCH 1/1] drivers/staging/pi433: New driver

Why is this all here in the patch body?  Usually git will strip this
out, but in the future, please don't put it here.

I'll try to queue this up in a few days and let's see what breaks :)

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1688206 — Re: [PATCH 1/1] drivers/staging/pi433: New driver

Hi Greg,
 
like I wrote before - unfortunally I couldn't find a git command, squashing
all my commits into one single patch. Therfore I copy and pasted the patch
manually.
 
Never the less, the first three rows were copied from a patch, originally
generated by git.
 
I used
git format-patch master --stdout -p > pi433_patch
for the first rows
and
git diff master > pi433_patch
fot the dif/patch itself.

If someone could help me with better git commands, I would be happy :-)
 
Thanks a lot,
 
Marcus

> Greg KH <gregkh@linuxfoundation.org> hat am 16. Juli 2017 um 12:15
> geschrieben:
>
>
> On Sun, Jul 16, 2017 at 11:52:32AM +0200, Wolf Entwicklungen wrote:
> > From: Marcus Wolf <linux@Wolf-Entwicklungen.de>
> > Date: Tue,16 Jul 2017 11:52:06 +0100
> > Subject: [PATCH 1/1] drivers/staging/pi433: New driver
>
> Why is this all here in the patch body? Usually git will strip this
> out, but in the future, please don't put it here.
>
> I'll try to queue this up in a few days and let's see what breaks :)
>
> thanks,
>
> greg k-h
>

[toc] | [prev] | [next] | [standalone]

#1688404 — Re: [PATCH 1/1] drivers/staging/pi433: New driver

On Sun, Jul 16, 2017 at 03:53:57PM +0200, Marcus Wolf wrote:
> Hi Greg,
>  
> like I wrote before - unfortunally I couldn't find a git command, squashing
> all my commits into one single patch. Therfore I copy and pasted the patch
> manually.

git diff FIRST_PATCH..HEAD > my.patch

will do it.

> Never the less, the first three rows were copied from a patch, originally
> generated by git.
>  
> I used
> git format-patch master --stdout -p > pi433_patch
> for the first rows
> and
> git diff master > pi433_patch
> fot the dif/patch itself.
> 
> If someone could help me with better git commands, I would be happy :-)


The issue is that the header was in the email body, if you use git
send-email, it should not show up.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web