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

Groups > linux.kernel > #1692820 > unrolled thread

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

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: [PATCH 1/1] drivers/staging/pi433: New driver Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-20 13:30 +0200
    Re: [PATCH 1/1] drivers/staging/pi433: New driver Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-20 13:40 +0200
      Re: [PATCH 1/1] drivers/staging/pi433: New driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-20 14:30 +0200
        Test of [PATCH 1/1] staging: pi433: fix problem with division in  rf69_set_deviation Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-20 18:00 +0200
        [PATCH 1/1] staging: pi433: fix problem with division in  rf69_set_deviation Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-20 18:00 +0200
          Re: [PATCH 1/1] staging: pi433: fix problem with division in  rf69_set_deviation Greg KH <gregkh@linuxfoundation.org> - 2017-07-29 02:10 +0200
            Re: [PATCH 1/1] staging: pi433: fix problem with division in  rf69_set_deviation Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-29 11:00 +0200
              Re: [PATCH 1/1] staging: pi433: fix problem with division in  rf69_set_deviation Greg KH <gregkh@linuxfoundation.org> - 2017-07-30 00:30 +0200
                Re: [PATCH 1/1] staging: pi433: fix problem with division in  rf69_set_deviation Marcus Wolf <marcus.wolf@wolf-entwicklungen.de> - 2017-07-30 17:30 +0200
    Re: [PATCH 1/1] drivers/staging/pi433: New driver Greg KH <gregkh@linuxfoundation.org> - 2017-07-20 13:40 +0200

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

Hi Greg,

below you can see the report with the error on m68k. It was sent to me from a
test robot of intel.

According to my research, this problem occured, because there is no hardware
support for floating point on the m68k (or it was configured not to use it).
Therefore gcc uses an internal function, provided by libgcc. Obviously libgcc
wasn't linked...

I don't know how to come arround this problem by modifying my code (except
reducing the accuracy of the calculation by not using floating point).

Can we ignore the error or can I do something else?
Maybe a special include just in case of m68k can help??!?
I don't have an environment for building m68k.

Cheers,

Marcus

> kbuild test robot <lkp@intel.com> hat am 17. Juli 2017 um 15:26 geschrieben:
>
>
> Hi Marcus,
>
> [auto build test ERROR on staging/staging-testing]
> [also build test ERROR on v4.13-rc1 next-20170717]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Wolf-Entwicklungen/drivers-staging-pi433-New-driver/20170716-181617
> config: m68k-allyesconfig (attached as .config)
> compiler: m68k-linux-gcc (GCC) 4.9.0
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=m68k
>
> All errors (new ones prefixed by >>):
>
> drivers/staging/pi433/rf69.o: In function `rf69_set_frequency':
> >> rf69.c:(.text+0x9e2): undefined reference to `__udivdi3'
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation

[toc] | [next] | [standalone]

#1692825

On Thu, Jul 20, 2017 at 01:23:05PM +0200, Marcus Wolf wrote:
> Hi Greg,
> 
> below you can see the report with the error on m68k. It was sent to me from a
> test robot of intel.
> 
> According to my research, this problem occured, because there is no hardware
> support for floating point on the m68k (or it was configured not to use it).
> Therefore gcc uses an internal function, provided by libgcc. Obviously libgcc
> wasn't linked...
> 
> I don't know how to come arround this problem by modifying my code (except
> reducing the accuracy of the calculation by not using floating point).

I don't see any floating point?  You're not allowed to use floating
point in the kernel.

> 
> Can we ignore the error or can I do something else?
> Maybe a special include just in case of m68k can help??!?
> I don't have an environment for building m68k.
> 

I think the answer is to use div_u64() and div64_u64 instead of
do_div()?  Or you could just add a depend in the Kconfig.

regards,
dan carpenter

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

#1692877

Hi Dan,

On Thu, Jul 20, 2017 at 1:37 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Thu, Jul 20, 2017 at 01:23:05PM +0200, Marcus Wolf wrote:
>> below you can see the report with the error on m68k. It was sent to me from a
>> test robot of intel.
>>
>> According to my research, this problem occured, because there is no hardware
>> support for floating point on the m68k (or it was configured not to use it).
>> Therefore gcc uses an internal function, provided by libgcc. Obviously libgcc
>> wasn't linked...
>>
>> I don't know how to come arround this problem by modifying my code (except
>> reducing the accuracy of the calculation by not using floating point).
>
> I don't see any floating point?  You're not allowed to use floating
> point in the kernel.

Indeed. __udivdi3 is used for 64-bit by 32-bit division.

>> Can we ignore the error or can I do something else?
>> Maybe a special include just in case of m68k can help??!?
>> I don't have an environment for building m68k.

https://www.kernel.org/pub/tools/crosstool/

> I think the answer is to use div_u64() and div64_u64 instead of
> do_div()?

do_div() is fine
The link error is not caused by do_div(), but by not using do_div() where
needed.

> Or you could just add a depend in the Kconfig.

Depend on what? !M68K? It's gonna fail on several 32-bit platforms.
Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

#1693055 — Test of [PATCH 1/1] staging: pi433: fix problem with division in rf69_set_deviation

Hi!

Since I don't have an environment for m68k and I would appreciate not
having to set it up, I would be very happy, if someone could give
the patch, named in the subjext, a try on ARCH=m68k...

Thanks,

Marcus



> Geert Uytterhoeven <geert@linux-m68k.org> hat am 20. Juli 2017 um 14:27
> geschrieben:
>
>
> Hi Dan,
>
> On Thu, Jul 20, 2017 at 1:37 PM, Dan Carpenter <dan.carpenter@oracle.com>
> wrote:
> > On Thu, Jul 20, 2017 at 01:23:05PM +0200, Marcus Wolf wrote:
> >> below you can see the report with the error on m68k. It was sent to me from
> >> a
> >> test robot of intel.
> >>
> >> According to my research, this problem occured, because there is no
> >> hardware
> >> support for floating point on the m68k (or it was configured not to use
> >> it).
> >> Therefore gcc uses an internal function, provided by libgcc. Obviously
> >> libgcc
> >> wasn't linked...
> >>
> >> I don't know how to come arround this problem by modifying my code (except
> >> reducing the accuracy of the calculation by not using floating point).
> >
> > I don't see any floating point? You're not allowed to use floating
> > point in the kernel.
>
> Indeed. __udivdi3 is used for 64-bit by 32-bit division.
>
> >> Can we ignore the error or can I do something else?
> >> Maybe a special include just in case of m68k can help??!?
> >> I don't have an environment for building m68k.
>
> https://www.kernel.org/pub/tools/crosstool/
>
> > I think the answer is to use div_u64() and div64_u64 instead of
> > do_div()?
>
> do_div() is fine
> The link error is not caused by do_div(), but by not using do_div() where
> needed.
>
> > Or you could just add a depend in the Kconfig.
>
> Depend on what? !M68K? It's gonna fail on several 32-bit platforms.
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
>

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

#1693058 — [PATCH 1/1] staging: pi433: fix problem with division in rf69_set_deviation

Fixes problem with division in rf69_set_deviation

Fixes: 874bcba65f9a ("staging: pi433: New driver")
Signed-off-by: Marcus Wolf <linux@wolf-entwicklungen.de>

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -221,7 +221,7 @@ int rf69_set_frequency(struct spi_device *spi, u32
frequency)
 int rf69_set_frequency(struct spi_device *spi, u32 frequency)
 {
 	int retval;
-	u32 f_max;
+	u64 f_max;
 	u64 f_reg;
 	u64 f_step;
 	u8 msb;
@@ -238,7 +238,8 @@ int rf69_set_frequency(struct spi_device *spi, u32
frequency)
 	do_div(f_step, 524288); //  524288 = 2^19
 
 	// check input value
-	f_max = f_step * 8388608 / factor;
+	f_max = f_step * 8388608;
+	do_div(f_max, factor);
 	if (frequency > f_max)
 	{
 		dev_dbg(&spi->dev, "setFrequency: illegal input param");

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

#1699184 — Re: [PATCH 1/1] staging: pi433: fix problem with division in rf69_set_deviation

On Thu, Jul 20, 2017 at 05:56:36PM +0200, Marcus Wolf wrote:
> Fixes problem with division in rf69_set_deviation
> 
> Fixes: 874bcba65f9a ("staging: pi433: New driver")
> Signed-off-by: Marcus Wolf <linux@wolf-entwicklungen.de>
> 
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -221,7 +221,7 @@ int rf69_set_frequency(struct spi_device *spi, u32
> frequency)

Patch is line-wrapped and does not apply :(

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

#1699266 — Re: [PATCH 1/1] staging: pi433: fix problem with division in rf69_set_deviation

Hi Greg,
 
already had a discussion concerning that patch with Dan yesterday.
I really don't know what's going on there. I detached the patch once more from
my outbox and had a very close look in an editor and looked at it with a
difftool. In my outbox the patch is fine. I really don't knwo why it reaches you
crapped.
 
Since there are contsant problems with my patches, I will stop sending patches
for a while. As soon as I can find time to deeply confess with the tooling, I
will start over with trying.
 
Concerning this patch: You can use Arnds Patch from yesterday instead:
[PATCH] staging: pi433: use div_u64 for 64-bit division
It's a bit different to my patch, but according to yesterdays discussion, it
should also fix the problem.
 
Sorry for any inconvenience,
 
Marcus
 

> Greg KH <gregkh@linuxfoundation.org> hat am 29. Juli 2017 um 02:01
> geschrieben:
>
>
> On Thu, Jul 20, 2017 at 05:56:36PM +0200, Marcus Wolf wrote:
> > Fixes problem with division in rf69_set_deviation
> >
> > Fixes: 874bcba65f9a ("staging: pi433: New driver")
> > Signed-off-by: Marcus Wolf <linux@wolf-entwicklungen.de>
> >
> > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> > --- a/drivers/staging/pi433/rf69.c
> > +++ b/drivers/staging/pi433/rf69.c
> > @@ -221,7 +221,7 @@ int rf69_set_frequency(struct spi_device *spi, u32
> > frequency)
>
> Patch is line-wrapped and does not apply :(
>

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

#1699387 — Re: [PATCH 1/1] staging: pi433: fix problem with division in rf69_set_deviation

On Sat, Jul 29, 2017 at 10:51:15AM +0200, Marcus Wolf wrote:
> Hi Greg,
>  
> already had a discussion concerning that patch with Dan yesterday.
> I really don't know what's going on there. I detached the patch once more from
> my outbox and had a very close look in an editor and looked at it with a
> difftool. In my outbox the patch is fine. I really don't knwo why it reaches you
> crapped.
>  
> Since there are contsant problems with my patches, I will stop sending patches
> for a while. As soon as I can find time to deeply confess with the tooling, I
> will start over with trying.

Just use 'git send-email' for patches if you are having problems with
your email client.  And get a better email client the kernel
Documentation has a whole file just about that topic and how to do it
correctly.

> Concerning this patch: You can use Arnds Patch from yesterday instead:
> [PATCH] staging: pi433: use div_u64 for 64-bit division
> It's a bit different to my patch, but according to yesterdays discussion, it
> should also fix the problem.

I already took it into my tree.

thanks,

greg k-h

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

#1699548 — Re: [PATCH 1/1] staging: pi433: fix problem with division in rf69_set_deviation

Hi Greg, hi Dan

the toolingproblem is a little bit more complicated. Since a mainline kernel
isn't directly able to boot on the RasPi, for testing I work with a Raspbian
kernel. Since at the beginning, I never thought of submitting the driver, I
started with a side-build project with SVN - that's stil my master... In
addition, disk is full :-( 

So in principle nothing is really adequate to meet the need of easy
participating in kernel development.
I will revise my tooling, as soon as I find the time and quietness - most
probably I need to setup from scratch.

For the announced change in rf69.c I will try to equip an SVN-diff with
apropriate headers next week, so Dan can crosscheck with his work - I hope, I
will make it without errors...

Have a nice weekend,

Marcus


> Greg KH <gregkh@linuxfoundation.org> hat am 30. Juli 2017 um 00:21
> geschrieben:
>
>
> On Sat, Jul 29, 2017 at 10:51:15AM +0200, Marcus Wolf wrote:
> > Hi Greg,
> >
> > already had a discussion concerning that patch with Dan yesterday.
> > I really don't know what's going on there. I detached the patch once more
> > from
> > my outbox and had a very close look in an editor and looked at it with a
> > difftool. In my outbox the patch is fine. I really don't knwo why it reaches
> > you
> > crapped.
> >
> > Since there are contsant problems with my patches, I will stop sending
> > patches
> > for a while. As soon as I can find time to deeply confess with the tooling,
> > I
> > will start over with trying.
>
> Just use 'git send-email' for patches if you are having problems with
> your email client. And get a better email client the kernel
> Documentation has a whole file just about that topic and how to do it
> correctly.
>
> > Concerning this patch: You can use Arnds Patch from yesterday instead:
> > [PATCH] staging: pi433: use div_u64 for 64-bit division
> > It's a bit different to my patch, but according to yesterdays discussion, it
> > should also fix the problem.
>
> I already took it into my tree.
>
> thanks,
>
> greg k-h
>

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

#1692837

On Thu, Jul 20, 2017 at 01:23:05PM +0200, Marcus Wolf wrote:
> Hi Greg,
> 
> below you can see the report with the error on m68k. It was sent to me from a
> test robot of intel.
> 
> According to my research, this problem occured, because there is no hardware
> support for floating point on the m68k (or it was configured not to use it).
> Therefore gcc uses an internal function, provided by libgcc. Obviously libgcc
> wasn't linked...
> 
> I don't know how to come arround this problem by modifying my code (except
> reducing the accuracy of the calculation by not using floating point).
> 
> Can we ignore the error or can I do something else?
> Maybe a special include just in case of m68k can help??!?
> I don't have an environment for building m68k.

It looks like a 64bit issue, try wrapping your division in that function
with a call to do_div() like you do elsewhere in that function.  That
should solve the issue.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web