Hi all,
After merging the net-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:
ERROR: "clk_get_rate" [drivers/net/can/c_can/c_can_pci.ko] undefined!
ERROR: "clk_get" [drivers/net/can/c_can/c_can_pci.ko] undefined!
ERROR: "clk_put" [drivers/net/can/c_can/c_can_pci.ko] undefined!
Caused by commit 5b92da0443c2 ("c_can_pci: generic module for C_CAN/D_CAN
on PCI"). Maybe a missing Kconfig dependency/select?
I have used the net-next tree from next-20120619 for today.
--
Cheers,
Stephen Rothwell [email protected]
From: Stephen Rothwell <[email protected]>
Date: Wed, 20 Jun 2012 13:33:48 +1000
> After merging the net-next tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> ERROR: "clk_get_rate" [drivers/net/can/c_can/c_can_pci.ko] undefined!
> ERROR: "clk_get" [drivers/net/can/c_can/c_can_pci.ko] undefined!
> ERROR: "clk_put" [drivers/net/can/c_can/c_can_pci.ko] undefined!
>
> Caused by commit 5b92da0443c2 ("c_can_pci: generic module for C_CAN/D_CAN
> on PCI"). Maybe a missing Kconfig dependency/select?
>
> I have used the net-next tree from next-20120619 for today.
Known problem:
http://marc.info/?l=linux-netdev&m=134014347620836&w=2
Hi,
> -----Original Message-----
> From: Stephen Rothwell [mailto:[email protected]]
> Sent: Wednesday, June 20, 2012 9:04 AM
> To: David Miller; [email protected]
> Cc: [email protected]; [email protected]; Federico
> Vaga; Giancarlo ASNAGHI; Wolfgang Grandegger; Bhupesh SHARMA; Marc
> Kleine-Budde
> Subject: linux-next: build failure after merge of the net-next tree
>
> Hi all,
>
> After merging the net-next tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> ERROR: "clk_get_rate" [drivers/net/can/c_can/c_can_pci.ko] undefined!
> ERROR: "clk_get" [drivers/net/can/c_can/c_can_pci.ko] undefined!
> ERROR: "clk_put" [drivers/net/can/c_can/c_can_pci.ko] undefined!
>
> Caused by commit 5b92da0443c2 ("c_can_pci: generic module for
> C_CAN/D_CAN on PCI"). Maybe a missing Kconfig dependency/select?
>
> I have used the net-next tree from next-20120619 for today.
> --
clk_get/clk_put* variants are usually used by ARM platforms.
Protecting their calls under macro 'CONFIG_HAVE_CLK' should solve the problem.
See [1] for how it is done in c_can_platform.c
Could you possibly add these checks and send a patch for the same?
It should be fairly simple.
[1] http://lxr.linux.no/linux+v3.4.3/drivers/net/can/c_can/c_can_platform.c#L68
Regards,
Bhupesh
From: Bhupesh SHARMA <[email protected]>
Date: Wed, 20 Jun 2012 12:27:11 +0800
> clk_get/clk_put* variants are usually used by ARM platforms.
> Protecting their calls under macro 'CONFIG_HAVE_CLK' should solve the problem.
No, we don't pepper foo.c files with crappy ifdefs.
Hi David,
> -----Original Message-----
> From: David Miller [mailto:[email protected]]
> Sent: Wednesday, June 20, 2012 10:07 AM
> To: Bhupesh SHARMA
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; Giancarlo ASNAGHI; [email protected];
> [email protected]
> Subject: Re: linux-next: build failure after merge of the net-next tree
>
> From: Bhupesh SHARMA <[email protected]>
> Date: Wed, 20 Jun 2012 12:27:11 +0800
>
> > clk_get/clk_put* variants are usually used by ARM platforms.
> > Protecting their calls under macro 'CONFIG_HAVE_CLK' should solve the
> problem.
>
> No, we don't pepper foo.c files with crappy ifdefs.
So, whether adding a check in Kconfig for HAVE_CLK be a proper solution ?
But that will limit the compilation of this driver for only platforms which are ARM based.
One may need to support this driver on x86 like platforms also..
Regards,
Bhupesh
From: Bhupesh SHARMA <[email protected]>
Date: Wed, 20 Jun 2012 12:45:46 +0800
> So, whether adding a check in Kconfig for HAVE_CLK be a proper
> solution ? But that will limit the compilation of this driver for
> only platforms which are ARM based.
>
> One may need to support this driver on x86 like platforms also..
Then x86 will need to provide clock operations, or there needs to
be dummy ones for such platforms.
This isn't rocket science.
On 20/06/12 05:27, Bhupesh SHARMA wrote:
>> -----Original Message-----
>> From: Stephen Rothwell [mailto:[email protected]]
>> Sent: Wednesday, June 20, 2012 9:04 AM
>> To: David Miller; [email protected]
>> Cc: [email protected]; [email protected]; Federico
>> Vaga; Giancarlo ASNAGHI; Wolfgang Grandegger; Bhupesh SHARMA; Marc
>> Kleine-Budde
>> Subject: linux-next: build failure after merge of the net-next tree
>>
>> Hi all,
>>
>> After merging the net-next tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> ERROR: "clk_get_rate" [drivers/net/can/c_can/c_can_pci.ko] undefined!
>> ERROR: "clk_get" [drivers/net/can/c_can/c_can_pci.ko] undefined!
>> ERROR: "clk_put" [drivers/net/can/c_can/c_can_pci.ko] undefined!
>>
>> Caused by commit 5b92da0443c2 ("c_can_pci: generic module for
>> C_CAN/D_CAN on PCI"). Maybe a missing Kconfig dependency/select?
>>
>> I have used the net-next tree from next-20120619 for today.
>> --
>
> clk_get/clk_put* variants are usually used by ARM platforms.
> Protecting their calls under macro 'CONFIG_HAVE_CLK' should solve the problem.
>
> See [1] for how it is done in c_can_platform.c
>
> Could you possibly add these checks and send a patch for the same?
> It should be fairly simple.
Hi Bhupesh,
Please see following patchset from me, that got applied in linux-next
https://lkml.org/lkml/2012/4/24/154
Please check if this patchset is present in your build repo. I believe it should be
there. If it is, then you shouldn't get these errors.
--
viresh
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
From: viresh kumar <[email protected]>
Date: Wed, 20 Jun 2012 09:08:34 +0100
> Please see following patchset from me, that got applied in linux-next
>
> https://lkml.org/lkml/2012/4/24/154
>
> Please check if this patchset is present in your build repo. I believe it should be
> there. If it is, then you shouldn't get these errors.
Well, then Stephen shouldn't get those errors either.
But obviously he did.
But all of this talk about changes existing only in linux-next is
entirely moot. Because The damn thing MUST build independently inside
of net-next which doesn't have those clock layer changes.
Someone send me a clean fix for net-next now.
On 06/20/2012 06:45 AM, Bhupesh SHARMA wrote:
> Hi David,
>
>> -----Original Message-----
>> From: David Miller [mailto:[email protected]]
>> Sent: Wednesday, June 20, 2012 10:07 AM
>> To: Bhupesh SHARMA
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]; [email protected];
>> [email protected]; Giancarlo ASNAGHI; [email protected];
>> [email protected]
>> Subject: Re: linux-next: build failure after merge of the net-next tree
>>
>> From: Bhupesh SHARMA <[email protected]>
>> Date: Wed, 20 Jun 2012 12:27:11 +0800
>>
>>> clk_get/clk_put* variants are usually used by ARM platforms.
>>> Protecting their calls under macro 'CONFIG_HAVE_CLK' should solve the
>> problem.
>>
>> No, we don't pepper foo.c files with crappy ifdefs.
>
> So, whether adding a check in Kconfig for HAVE_CLK be a proper solution ?
> But that will limit the compilation of this driver for only platforms which are ARM based.
>
> One may need to support this driver on x86 like platforms also..
I think the driver is currently used/tested only on ARM only, so what
about adding a HAVE_CLK dependency, until Viresh Kumar's patches are
available in net-next. Later we can remove it.
regards, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
From: Marc Kleine-Budde <[email protected]>
Date: Wed, 20 Jun 2012 10:31:26 +0200
> I think the driver is currently used/tested only on ARM only, so what
> about adding a HAVE_CLK dependency, until Viresh Kumar's patches are
> available in net-next. Later we can remove it.
Sounds good.
On 20/06/12 09:20, David Miller wrote:
> From: viresh kumar <[email protected]>
> Date: Wed, 20 Jun 2012 09:08:34 +0100
>
>> > Please see following patchset from me, that got applied in linux-next
>> >
>> > https://lkml.org/lkml/2012/4/24/154
>> >
>> > Please check if this patchset is present in your build repo. I believe it should be
>> > there. If it is, then you shouldn't get these errors.
> Well, then Stephen shouldn't get those errors either.
>
> But obviously he did.
Surprised. I am not able to get why this happen. Because we don't have HAVE_CLK defined
for x86_64, then the inline routines like following should come into play:
static inline unsigned long clk_get_rate(struct clk *clk)
{
return 0;
}
Is there any tricky part for such routines with drivers compiled as modules?
--
Viresh
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> I think the driver is currently used/tested only on ARM only, so what
> about adding a HAVE_CLK dependency, until Viresh Kumar's patches are
> available in net-next. Later we can remove it.
This driver is tested only on an x86 with AMBA peripherals on PCI. We
are developing the clk side for this new platform.
--
Federico Vaga
From: Federico Vaga <[email protected]>
Date: Wed, 20 Jun 2012 11:03:08 +0200
>> I think the driver is currently used/tested only on ARM only, so what
>> about adding a HAVE_CLK dependency, until Viresh Kumar's patches are
>> available in net-next. Later we can remove it.
>
> This driver is tested only on an x86 with AMBA peripherals on PCI. We
> are developing the clk side for this new platform.
Then the driver should NEVER have been submitted without the
required infrastructure in place first.
If what you say is true, I'm extremely pissed off about this now.
This submission was a complete cluster fuck, and it means the person
who submitted this to me didn't test the damn build even.
> Then the driver should NEVER have been submitted without the
> required infrastructure in place first.
This particular driver don't use the clk framework at the moment. I put
that lines about clk to try to be generic as possibile, but I see that I
made a mistake: I'm sorry. An alternative solution to HAVE_CLK
dependency can be: remove the clk_* lines because actualy nobody use
them. In the future, if our c_can migrate to clk and our clk framework
is accepted in the kernel, we can re-add the clk_* lines.
--
Federico Vaga
From: Federico Vaga <[email protected]>
Date: Wed, 20 Jun 2012 11:59:26 +0200
>> Then the driver should NEVER have been submitted without the
>> required infrastructure in place first.
>
> This particular driver don't use the clk framework at the moment. I put
> that lines about clk to try to be generic as possibile, but I see that I
> made a mistake: I'm sorry.
Why would you try to be generic by using an interface currently
only available on certain platforms?
That is how you make drivers non-portable, and not generic.
> Why would you try to be generic by using an interface currently
> only available on certain platforms?
I know, I was wrong.
> That is how you make drivers non-portable, and not generic.
Now is fixed in my mind; I learn the lesson.
--
Federico Vaga
Hi all,
On Wed, 20 Jun 2012 01:20:37 -0700 (PDT) David Miller <[email protected]> wrote:
>
> From: viresh kumar <[email protected]>
> Date: Wed, 20 Jun 2012 09:08:34 +0100
>
> > Please see following patchset from me, that got applied in linux-next
> >
> > https://lkml.org/lkml/2012/4/24/154
> >
> > Please check if this patchset is present in your build repo. I believe it should be
> > there. If it is, then you shouldn't get these errors.
>
> Well, then Stephen shouldn't get those errors either.
>
> But obviously he did.
>
> But all of this talk about changes existing only in linux-next is
> entirely moot. Because The damn thing MUST build independently inside
> of net-next which doesn't have those clock layer changes.
>
> Someone send me a clean fix for net-next now.
I get those errors because those patches are in the akpm tree which is
merged after everything else ...
One possibility is to put those changes in another (stable) tree and
merge that into the net-next tree (and any other tree that needs it).
--
Cheers,
Stephen Rothwell [email protected]
On 06/20/2012 12:26 PM, Stephen Rothwell wrote:
> Hi all,
>
> On Wed, 20 Jun 2012 01:20:37 -0700 (PDT) David Miller <[email protected]> wrote:
>>
>> From: viresh kumar <[email protected]>
>> Date: Wed, 20 Jun 2012 09:08:34 +0100
>>
>>> Please see following patchset from me, that got applied in linux-next
>>>
>>> https://lkml.org/lkml/2012/4/24/154
>>>
>>> Please check if this patchset is present in your build repo. I believe it should be
>>> there. If it is, then you shouldn't get these errors.
>>
>> Well, then Stephen shouldn't get those errors either.
>>
>> But obviously he did.
>>
>> But all of this talk about changes existing only in linux-next is
>> entirely moot. Because The damn thing MUST build independently inside
>> of net-next which doesn't have those clock layer changes.
>>
>> Someone send me a clean fix for net-next now.
>
> I get those errors because those patches are in the akpm tree which is
> merged after everything else ...
>
> One possibility is to put those changes in another (stable) tree and
> merge that into the net-next tree (and any other tree that needs it).
We're about to remove the offending clk_*() functions from the driver,
as they are untested anyway. The hardware the driver was developed for
uses a hardcoded clock rate in the driver anyway, as it cannot be
retrieved from clock tree. As soon as there is hardware available that
will work with the clock tree, we can add those functions back.
Sorry for the noise,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
On Tue, Jun 19, 2012 at 09:47:59PM -0700, David Miller wrote:
> From: Bhupesh SHARMA <[email protected]>
> > So, whether adding a check in Kconfig for HAVE_CLK be a proper
> > solution ? But that will limit the compilation of this driver for
> > only platforms which are ARM based.
> > One may need to support this driver on x86 like platforms also..
> Then x86 will need to provide clock operations, or there needs to
> be dummy ones for such platforms.
Not directly germane but I've been sending patches for this to the x86
guys for a little while though they're doing a /dev/null impression.
> This isn't rocket science.
The other option is that the clock API stubs itself out when not enabled
which is going into mainline (not sure quite where it is at the minute).
These sort of per-arch APIs should be a legacy thing, hopefully we'll
manage to squash them at some point and we should certainly work to
avoid introducing new ones.
On 09/28/2012 04:19 AM, David Miller wrote:
> From: Stephen Rothwell <[email protected]>
> Date: Fri, 28 Sep 2012 11:43:35 +1000
>
>> Hi all,
>>
>> After merging the net-next tree, today's linux-next build (powerpc
>> ppc64_defconfig) failed like this:
>>
>> drivers/net/ethernet/emulex/benet/be_main.c: In function 'be_find_vfs':
>> drivers/net/ethernet/emulex/benet/be_main.c:1090:28: error: 'struct pci_dev' has no member named 'physfn'
>>
>> Caused by commit 51af6d7c1f31 ("be2net: fix vfs enumeration"). physfn is
>> only defined if CONFIG_PCI_ATS is set.
>>
>> I have reverted that commit for today.
>
> I'm reverting it too, thanks for reporting Stephen.
>
Sorry for that, it will be better to use pci_physfn to access this
member. I'll repost the patch.
Ivan