* [PATCH v3 24/27] can: replace devm_ioremap_nocache with devm_ioremap
From: Yisheng Xie @ 2017-12-23 11:02 UTC (permalink / raw)
To: linux-kernel, gregkh
Cc: ysxie, Yisheng Xie, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can, netdev
Default ioremap is ioremap_nocache, so devm_ioremap has the same
function with devm_ioremap_nocache, which can just be killed to
save the size of devres.o
This patch is to use use devm_ioremap instead of devm_ioremap_nocache,
which should not have any function change but prepare for killing
devm_ioremap_nocache.
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
drivers/net/can/sja1000/sja1000_platform.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index dc9c6db..c59500c 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -240,8 +240,8 @@ static int sp_probe(struct platform_device *pdev)
resource_size(res_mem), DRV_NAME))
return -EBUSY;
- addr = devm_ioremap_nocache(&pdev->dev, res_mem->start,
- resource_size(res_mem));
+ addr = devm_ioremap(&pdev->dev, res_mem->start,
+ resource_size(res_mem));
if (!addr)
return -ENOMEM;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 27/27] devres: kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-23 11:02 UTC (permalink / raw)
To: linux-kernel, gregkh
Cc: ysxie, ulf.hansson, linux-mmc, boris.brezillon, richard,
marek.vasut, cyrille.pitchen, linux-mtd, alsa-devel, wim, linux,
linux-watchdog, b.zolnierkie, linux-fbdev, linus.walleij,
linux-gpio, ralf, linux-mips, lgirdwood, broonie, tglx, jason,
marc.zyngier, arnd, andriy.shevchenko, industrypack-devel, wg,
mkl, linux-can, mcheh
Now, nobody use devm_ioremap_nocache anymore, can it can just be
removed. After this patch the size of devres.o will be reduced from
20304 bytes to 18992 bytes.
Suggested-by: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
Documentation/driver-model/devres.txt | 1 -
include/linux/io.h | 2 --
lib/devres.c | 29 -----------------------------
scripts/coccinelle/free/devm_free.cocci | 2 --
4 files changed, 34 deletions(-)
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index c180045..c3fddb5 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -292,7 +292,6 @@ IOMAP
devm_ioport_map()
devm_ioport_unmap()
devm_ioremap()
- devm_ioremap_nocache()
devm_ioremap_wc()
devm_ioremap_resource() : checks resource, requests memory region, ioremaps
devm_iounmap()
diff --git a/include/linux/io.h b/include/linux/io.h
index 32e30e8..a9c7270 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -75,8 +75,6 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
resource_size_t size);
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
- resource_size_t size);
void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
resource_size_t size);
void devm_iounmap(struct device *dev, void __iomem *addr);
diff --git a/lib/devres.c b/lib/devres.c
index 5f2aedd..f818fcf 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
EXPORT_SYMBOL(devm_ioremap);
/**
- * devm_ioremap_nocache - Managed ioremap_nocache()
- * @dev: Generic device to remap IO address for
- * @offset: Resource address to map
- * @size: Size of map
- *
- * Managed ioremap_nocache(). Map is automatically unmapped on driver
- * detach.
- */
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
- resource_size_t size)
-{
- void __iomem **ptr, *addr;
-
- ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- addr = ioremap_nocache(offset, size);
- if (addr) {
- *ptr = addr;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return addr;
-}
-EXPORT_SYMBOL(devm_ioremap_nocache);
-
-/**
* devm_ioremap_wc - Managed ioremap_wc()
* @dev: Generic device to remap IO address for
* @offset: Resource address to map
diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index c990d2c..36b8752 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -51,8 +51,6 @@ expression x;
|
x = devm_ioremap(...)
|
- x = devm_ioremap_nocache(...)
-|
x = devm_ioport_map(...)
)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v3 27/27] devres: kill devm_ioremap_nocache
From: Greg KH @ 2017-12-23 13:45 UTC (permalink / raw)
To: Yisheng Xie
Cc: linux-mips, ulf.hansson, jakub.kicinski, lgirdwood, airlied,
linux-pci, alsa-devel, dri-devel, platform-driver-x86, linux-ide,
linux-mtd, daniel.vetter, tglx, linux-watchdog, linux-rtc,
boris.brezillon, andriy.shevchenko, vinod.koul, richard,
alexandre.belloni, marek.vasut, industrypack-devel, jslaby,
dvhart, linux, linux-media, devel, jason, arnd, b.zolnierkie,
marc.zyngier, linux-mmc, linux-can, linux-gp
In-Reply-To: <1514026979-33838-1-git-send-email-xieyisheng1@huawei.com>
On Sat, Dec 23, 2017 at 07:02:59PM +0800, Yisheng Xie wrote:
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> EXPORT_SYMBOL(devm_ioremap);
>
> /**
> - * devm_ioremap_nocache - Managed ioremap_nocache()
> - * @dev: Generic device to remap IO address for
> - * @offset: Resource address to map
> - * @size: Size of map
> - *
> - * Managed ioremap_nocache(). Map is automatically unmapped on driver
> - * detach.
> - */
> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> - resource_size_t size)
> -{
> - void __iomem **ptr, *addr;
> -
> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> -
> - addr = ioremap_nocache(offset, size);
Wait, devm_ioremap() calls ioremap(), not ioremap_nocache(), are you
_SURE_ that these are all identical? For all arches? If so, then
ioremap_nocache() can also be removed, right?
In my quick glance, I don't think you can do this series at all :(
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: Greg KH @ 2017-12-23 13:48 UTC (permalink / raw)
To: Yisheng Xie
Cc: linux-mips, ulf.hansson, jakub.kicinski, platform-driver-x86,
airlied, linux-wireless, linus.walleij, alsa-devel, dri-devel,
linux-kernel, linux-ide, linux-mtd, daniel.vetter, dan.j.williams,
jason, linux-rtc, boris.brezillon, mchehab, dmaengine, vinod.koul,
richard, marek.vasut, industrypack-devel, linux-pci, dvhart,
linux, linux-media, seanpaul, devel, linux-watchdog, arnd,
b.zolnierkie, marc.zyngier, jslaby
In-Reply-To: <1514026525-32538-1-git-send-email-xieyisheng1@huawei.com>
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
> Hi all,
>
> When I tried to use devm_ioremap function and review related code, I found
> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
> except one use ioremap while the other use ioremap_nocache.
For all arches? Really? Look at MIPS, and x86, they have different
functions.
> While ioremap's
> default function is ioremap_nocache, so devm_ioremap_nocache also have the
> same function with devm_ioremap, which can just be killed to reduce the size
> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>
> I have posted two versions, which use macro instead of function for
> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
> devm_ioremap_nocache for no need to keep a macro around for the duplicate
> thing. So here comes v3 and please help to review.
I don't think this can be done, what am I missing? These functions are
not identical, sorry for missing that before.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: Guenter Roeck @ 2017-12-23 15:57 UTC (permalink / raw)
To: Greg KH, Yisheng Xie
Cc: linux-mips, ulf.hansson, jakub.kicinski, platform-driver-x86,
airlied, linux-wireless, linus.walleij, alsa-devel, dri-devel,
linux-kernel, linux-ide, linux-mtd, daniel.vetter, dan.j.williams,
jason, linux-rtc, boris.brezillon, mchehab, dmaengine, vinod.koul,
richard, marek.vasut, industrypack-devel, linux-pci, dvhart, wg,
linux-media, seanpaul, devel, linux-watchdog, arnd, b.zolnierkie,
marc.zyngier, jslaby
In-Reply-To: <20171223134831.GB10103@kroah.com>
On 12/23/2017 05:48 AM, Greg KH wrote:
> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>> Hi all,
>>
>> When I tried to use devm_ioremap function and review related code, I found
>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>> except one use ioremap while the other use ioremap_nocache.
>
> For all arches? Really? Look at MIPS, and x86, they have different
> functions.
>
Both mips and x86 end up mapping the same function, but other arches don't.
mn10300 is one where ioremap and ioremap_nocache are definitely different.
Guenter
>> While ioremap's
>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>> same function with devm_ioremap, which can just be killed to reduce the size
>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>
>> I have posted two versions, which use macro instead of function for
>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>> thing. So here comes v3 and please help to review.
>
> I don't think this can be done, what am I missing? These functions are
> not identical, sorry for missing that before.
>
> thanks,
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: christophe leroy @ 2017-12-24 8:55 UTC (permalink / raw)
To: Guenter Roeck, Greg KH, Yisheng Xie
Cc: linux-kernel, ysxie, ulf.hansson, linux-mmc, boris.brezillon,
richard, marek.vasut, cyrille.pitchen, linux-mtd, alsa-devel, wim,
linux-watchdog, b.zolnierkie, linux-fbdev, linus.walleij,
linux-gpio, ralf, linux-mips, lgirdwood, broonie, tglx, jason,
marc.zyngier, arnd, andriy.shevchenko, industrypack-devel, wg,
mkl, linux-can, mchehab, linux-media, a.zum
In-Reply-To: <f7632cf5-2bcc-4d74-b912-3999937a1269@roeck-us.net>
Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
> On 12/23/2017 05:48 AM, Greg KH wrote:
>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>> Hi all,
>>>
>>> When I tried to use devm_ioremap function and review related code, I
>>> found
>>> devm_ioremap and devm_ioremap_nocache is almost the same with each
>>> other,
>>> except one use ioremap while the other use ioremap_nocache.
>>
>> For all arches? Really? Look at MIPS, and x86, they have different
>> functions.
>>
>
> Both mips and x86 end up mapping the same function, but other arches don't.
> mn10300 is one where ioremap and ioremap_nocache are definitely different.
alpha: identical
arc: identical
arm: identical
arm64: identical
cris: different <==
frv: identical
hexagone: identical
ia64: different <==
m32r: identical
m68k: identical
metag: identical
microblaze: identical
mips: identical
mn10300: different <==
nios: identical
openrisc: different <==
parisc: identical
riscv: identical
s390: identical
sh: identical
sparc: identical
tile: identical
um: rely on asm/generic
unicore32: identical
x86: identical
asm/generic (no mmu): identical
So 4 among all arches seems to have ioremap() and ioremap_nocache()
being different.
Could we have a define set by the 4 arches on which ioremap() and
ioremap_nocache() are different, something like
HAVE_DIFFERENT_IOREMAP_NOCACHE ?
Christophe
>
> Guenter
>
>>> While ioremap's
>>> default function is ioremap_nocache, so devm_ioremap_nocache also
>>> have the
>>> same function with devm_ioremap, which can just be killed to reduce
>>> the size
>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>
>>> I have posted two versions, which use macro instead of function for
>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>> devm_ioremap_nocache for no need to keep a macro around for the
>>> duplicate
>>> thing. So here comes v3 and please help to review.
>>
>> I don't think this can be done, what am I missing? These functions are
>> not identical, sorry for missing that before.
>>
>> thanks,
>>
>> greg k-h
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: christophe leroy @ 2017-12-24 9:05 UTC (permalink / raw)
To: Greg KH, Yisheng Xie
Cc: linux-mips, ulf.hansson, jakub.kicinski, platform-driver-x86,
airlied, linux-wireless, linus.walleij, alsa-devel, dri-devel,
linux-kernel, linux-ide, linux-mtd, daniel.vetter, dan.j.williams,
jason, linux-rtc, boris.brezillon, mchehab, dmaengine, vinod.koul,
richard, marek.vasut, industrypack-devel, linux-pci, dvhart,
linux, linux-media, seanpaul, devel, linux-watchdog, arnd,
b.zolnierkie, marc.zyngier, jslaby
In-Reply-To: <20171223134831.GB10103@kroah.com>
Le 23/12/2017 à 14:48, Greg KH a écrit :
> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>> Hi all,
>>
>> When I tried to use devm_ioremap function and review related code, I found
>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>> except one use ioremap while the other use ioremap_nocache.
>
> For all arches? Really? Look at MIPS, and x86, they have different
> functions.
>
>> While ioremap's
>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>> same function with devm_ioremap, which can just be killed to reduce the size
>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>
>> I have posted two versions, which use macro instead of function for
>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>> thing. So here comes v3 and please help to review.
>
> I don't think this can be done, what am I missing? These functions are
> not identical, sorry for missing that before.
devm_ioremap() and devm_ioremap_nocache() are quite similar, both use
devm_ioremap_release() for the release, why not just defining:
static void __iomem *__devm_ioremap(struct device *dev, resource_size_t
offset,
resource_size_t size, bool nocache)
{
[...]
if (nocache)
addr = ioremap_nocache(offset, size);
else
addr = ioremap(offset, size);
[...]
}
then in include/linux/io.h
static inline void __iomem *devm_ioremap(struct device *dev,
resource_size_t offset,
resource_size_t size)
{return __devm_ioremap(dev, offset, size, false);}
static inline void __iomem *devm_ioremap_nocache(struct device *dev,
resource_size_t offset,
resource_size_t size);
{return __devm_ioremap(dev, offset, size, true);}
Christophe
>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-25 1:09 UTC (permalink / raw)
To: christophe leroy, Guenter Roeck, Greg KH
Cc: linux-mips, ulf.hansson, jakub.kicinski, platform-driver-x86,
airlied, linux-wireless, linus.walleij, alsa-devel, dri-devel,
linux-kernel, linux-ide, linux-mtd, daniel.vetter, dan.j.williams,
jason, linux-rtc, boris.brezillon, mchehab, dmaengine, vinod.koul,
richard, marek.vasut, industrypack-devel, linux-pci, dvhart, wg,
linux-media, seanpaul, devel, linux-watchdog, arnd, b.zolnierkie,
marc.zyngier, jslaby
In-Reply-To: <c28ac0bc-8bd2-3dce-3167-8c0f80ec601e@c-s.fr>
hi Christophe and Greg,
On 2017/12/24 16:55, christophe leroy wrote:
>
>
> Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
>> On 12/23/2017 05:48 AM, Greg KH wrote:
>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>> Hi all,
>>>>
>>>> When I tried to use devm_ioremap function and review related code, I found
>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>>> except one use ioremap while the other use ioremap_nocache.
>>>
>>> For all arches? Really? Look at MIPS, and x86, they have different
>>> functions.
>>>
>>
>> Both mips and x86 end up mapping the same function, but other arches don't.
>> mn10300 is one where ioremap and ioremap_nocache are definitely different.
>
> alpha: identical
> arc: identical
> arm: identical
> arm64: identical
> cris: different <==
> frv: identical
> hexagone: identical
> ia64: different <==
> m32r: identical
> m68k: identical
> metag: identical
> microblaze: identical
> mips: identical
> mn10300: different <==
> nios: identical
> openrisc: different <==
> parisc: identical
> riscv: identical
> s390: identical
> sh: identical
> sparc: identical
> tile: identical
> um: rely on asm/generic
> unicore32: identical
> x86: identical
> asm/generic (no mmu): identical
Wow, that's correct, sorry for I have just checked the main archs, I means
x86,arm, arm64, mips.
However, I stall have no idea about why these 4 archs want different ioremap
function with others. Drivers seems cannot aware this? If driver call ioremap
want he really want for there 4 archs, cache or nocache?
>
> So 4 among all arches seems to have ioremap() and ioremap_nocache() being different.
>
> Could we have a define set by the 4 arches on which ioremap() and ioremap_nocache() are different, something like HAVE_DIFFERENT_IOREMAP_NOCACHE ?
Then, what the HAVE_DIFFERENT_IOREMAP_NOCACHE is uesed for ?
Thanks
Yisheng
>
> Christophe
>
>>
>> Guenter
>>
>>>> While ioremap's
>>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>>> same function with devm_ioremap, which can just be killed to reduce the size
>>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>>
>>>> I have posted two versions, which use macro instead of function for
>>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>>> thing. So here comes v3 and please help to review.
>>>
>>> I don't think this can be done, what am I missing? These functions are
>>> not identical, sorry for missing that before.
Never mind, I should checked all the arches, sorry about that.
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
> https://www.avast.com/antivirus
>
>
> .
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-25 1:34 UTC (permalink / raw)
To: christophe leroy, Greg KH
Cc: linux-mips, ulf.hansson, jakub.kicinski, platform-driver-x86,
airlied, linux-wireless, linus.walleij, alsa-devel, dri-devel,
linux-kernel, linux-ide, linux-mtd, daniel.vetter, dan.j.williams,
jason, linux-rtc, boris.brezillon, mchehab, dmaengine, vinod.koul,
richard, marek.vasut, industrypack-devel, linux-pci, dvhart,
linux, linux-media, seanpaul, devel, linux-watchdog, arnd,
b.zolnierkie, marc.zyngier, jslaby
In-Reply-To: <b8ff7f17-7f2c-f220-9833-7ae5bd7343d5@c-s.fr>
On 2017/12/24 17:05, christophe leroy wrote:
>
>
> Le 23/12/2017 à 14:48, Greg KH a écrit :
>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>> Hi all,
>>>
>>> When I tried to use devm_ioremap function and review related code, I found
>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>> except one use ioremap while the other use ioremap_nocache.
>>
>> For all arches? Really? Look at MIPS, and x86, they have different
>> functions.
>>
>>> While ioremap's
>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>> same function with devm_ioremap, which can just be killed to reduce the size
>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>
>>> I have posted two versions, which use macro instead of function for
>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>> thing. So here comes v3 and please help to review.
>>
>> I don't think this can be done, what am I missing? These functions are
>> not identical, sorry for missing that before.
>
> devm_ioremap() and devm_ioremap_nocache() are quite similar, both use devm_ioremap_release() for the release, why not just defining:
>
> static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
> resource_size_t size, bool nocache)
> {
> [...]
> if (nocache)
> addr = ioremap_nocache(offset, size);
> else
> addr = ioremap(offset, size);
> [...]
> }
>
> then in include/linux/io.h
>
> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> resource_size_t size)
> {return __devm_ioremap(dev, offset, size, false);}
>
> static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> resource_size_t size);
> {return __devm_ioremap(dev, offset, size, true);}
Yeah, this seems good to me, right now we have devm_ioremap, devm_ioremap_wc, devm_ioremap_nocache
May be we can use an enum like:
typedef enum {
DEVM_IOREMAP = 0,
DEVM_IOREMAP_NOCACHE,
DEVM_IOREMAP_WC,
} devm_ioremap_type;
static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
resource_size_t size)
{return __devm_ioremap(dev, offset, size, DEVM_IOREMAP);}
static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
resource_size_t size);
{return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NOCACHE);}
static inline void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
resource_size_t size);
{return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);}
static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
resource_size_t size, devm_ioremap_type type)
{
void __iomem **ptr, *addr = NULL;
[...]
switch (type){
case DEVM_IOREMAP:
addr = ioremap(offset, size);
break;
case DEVM_IOREMAP_NOCACHE:
addr = ioremap_nocache(offset, size);
break;
case DEVM_IOREMAP_WC:
addr = ioremap_wc(offset, size);
break;
}
[...]
}
Thanks
Yisheng
>
> Christophe
>
>>
>> thanks,
>>
>> greg k-h
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
> https://www.avast.com/antivirus
>
>
> .
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH v3 27/27] devres: kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-25 1:43 UTC (permalink / raw)
To: Greg KH
Cc: linux-mips, ulf.hansson, jakub.kicinski, lgirdwood, airlied,
linux-pci, linus.walleij, alsa-devel, dri-devel,
platform-driver-x86, linux-ide, linux-mtd, daniel.vetter, tglx,
linux-watchdog, linux-rtc, boris.brezillon, andriy.shevchenko,
vinod.koul, richard, alexandre.belloni, marek.vasut,
industrypack-devel, jslaby, dvhart, linux, linux-media, devel,
jason, arnd, b.zolnierkie, marc.zyngier, linux-mmc, jani.niku
In-Reply-To: <20171223134532.GA10103@kroah.com>
On 2017/12/23 21:45, Greg KH wrote:
> On Sat, Dec 23, 2017 at 07:02:59PM +0800, Yisheng Xie wrote:
>> --- a/lib/devres.c
>> +++ b/lib/devres.c
>> @@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>> EXPORT_SYMBOL(devm_ioremap);
>>
>> /**
>> - * devm_ioremap_nocache - Managed ioremap_nocache()
>> - * @dev: Generic device to remap IO address for
>> - * @offset: Resource address to map
>> - * @size: Size of map
>> - *
>> - * Managed ioremap_nocache(). Map is automatically unmapped on driver
>> - * detach.
>> - */
>> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>> - resource_size_t size)
>> -{
>> - void __iomem **ptr, *addr;
>> -
>> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
>> - if (!ptr)
>> - return NULL;
>> -
>> - addr = ioremap_nocache(offset, size);
>
> Wait, devm_ioremap() calls ioremap(), not ioremap_nocache(), are you
> _SURE_ that these are all identical? For all arches? If so, then
> ioremap_nocache() can also be removed, right?
Yeah, As Christophe pointed out, that 4 archs do not have the same function.
But I do not why they do not want do the same thing. Driver may no know about
this? right?
>
> In my quick glance, I don't think you can do this series at all :(
Yes, maybe should take Christophe suggestion and use a bool or enum to distinguish them?
Thanks
Yisheng
>
> greg k-h
>
> .
>
^ permalink raw reply
* Re: [PATCH] flex_can: Fix checking can_dlc
From: Oliver Hartkopp @ 2017-12-25 11:38 UTC (permalink / raw)
To: Luu An Phu, wg, mkl; +Cc: linux-can, netdev, stefan-gabriel.mirea
In-Reply-To: <1513672833-28402-1-git-send-email-phu.luuan@nxp.com>
This patch looks wrong to me.
On 12/19/2017 09:40 AM, Luu An Phu wrote:
> From: "phu.luuan" <phu.luuan@nxp.com>
>
> flexcan_start_xmit should write data to register when can_dlc > 4.
> Because can_dlc is data length and it has value from 1 to 8.
No. can_dlc can contain values from 0 to 8. Even 0 is a valid DLC.
> But CAN data
> index has value from 0 to 7. So in case we have data in cf->data[4],
> the can_dlc has value is more than 4.
>
> Signed-off-by: Luu An Phu <phu.luuan@nxp.com>
> ---
> drivers/net/can/flexcan.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 0626dcf..0e3ff13 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2009 Sascha Hauer, Pengutronix
> * Copyright (c) 2010-2017 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
> * Copyright (c) 2014 David Jander, Protonic Holland
> + * Copyright 2017 NXP
> *
> * Based on code originally by Andrey Volkov <avolkov@varma-el.com>
> *
> @@ -526,7 +527,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> data = be32_to_cpup((__be32 *)&cf->data[0]);
> flexcan_write(data, &priv->tx_mb->data[0]);
> }
> - if (cf->can_dlc > 3) {
This is correct as data[0 .. 3] are filled from the code above. And if
can_dlc is greater than 3 (== 4 .. 8) the following 4 bytes are copied
into the registers.
So everything is correct with the current code.
> + if (cf->can_dlc > 4) {
> data = be32_to_cpup((__be32 *)&cf->data[4]);
> flexcan_write(data, &priv->tx_mb->data[1]);
> }
>
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH] flex_can: Fix checking can_dlc
From: Oliver Hartkopp @ 2017-12-25 11:58 UTC (permalink / raw)
To: Luu An Phu, wg, mkl; +Cc: linux-can, netdev, stefan-gabriel.mirea
In-Reply-To: <486344fc-f4cf-6311-ab83-ecf9958d16e9@hartkopp.net>
Answering myself after reading my own comment once more:
In fact the code fix seems to be correct but the commit comment was
completely wrong which lead to my answer ...
can_dlc can hold values from 0 .. 8.
The first 4 bytes are placed in data[0..3]. When we have more(!) than 4
bytes in can_dlc, the bytes 5..8 are placed in data[4..7].
The good thing was, that the current check was more conservative than
your suggestion so that we did not detect any data loss.
Please send a V2 patch with corrected commit message.
Thanks,
Oliver
ps.
>> From: "phu.luuan" <phu.luuan@nxp.com>
>> + * Copyright 2017 NXP
A one-liner contribution like this tiny fix usually does not lead to an
attribution in the copyrights. Your contribution is already perfectly
recorded by the Signed-off-by tag.
^ permalink raw reply
* Re: You will definetely be interested...
From: Sra. Angel Rania @ 2017-12-27 10:16 UTC (permalink / raw)
Hi Dear,
Reading your profile has given me courage in search of a reasponsable
and trust worthy Fellow. The past has treated me so awfully but now I
am ready to move on despite of my health condition. I will like to
have a sincere and important discussion with you that will be in your
favor likewise to you and your environment especially to your close
family. Endeavor to reply me and I have attached my picture in case
you long to know who emailed you. I will be waiting to hear from you
as soon as possble.
Thanks for paying attention to my mail and will appreciate so much if
I receive a reply from you for understable details.
Thanks,
Mrs. Rania Hassan
^ permalink raw reply
* Re: [PATCH v6 0/6] Add M_CAN Support for Dra76 platform
From: Yang, Wenyou @ 2017-12-29 3:38 UTC (permalink / raw)
To: Faiz Abbas, wg, mkl, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, sergei.shtylyov
In-Reply-To: <1513949488-13026-1-git-send-email-faiz_abbas@ti.com>
On 2017/12/22 21:31, Faiz Abbas wrote:
> This patch series adds support for M_CAN on the TI Dra76
> platform. Device tree patches will be sent separately.
> A bunch of patches were sent before by
> Franklin Cooper <fcooper@ti.com>. I have clubbed the
> series together and rebased to the latest kernel.
Tested this series on SAMA5D2 Xplained board.
Tested-by: Wenyou Yang <wenyou.yang@microchip.com>
>
> v6 changes:
> Dropped the patches to make hclk optional. Drivers
> which enable hclk as the interface clock using
> pm_runtime calls must still provide a hclk in the
> clocks property.
>
> Support higher speed CAN-FD bitrate:
> The community decided that data sampling point be used
> for the secondary sampling point here
> https://patchwork.kernel.org/patch/9909845/
>
> Franklin S Cooper Jr (6):
> can: dev: Add support for limiting configured bitrate
> can: m_can: Add call to of_can_transceiver
> can: m_can: Add PM Runtime
> can: m_can: Support higher speed CAN-FD bitrates
> dt-bindings: can: m_can: Document new can transceiver binding
> dt-bindings: can: can-transceiver: Document new binding
>
> .../bindings/net/can/can-transceiver.txt | 24 +++++++
> .../devicetree/bindings/net/can/m_can.txt | 9 +++
> drivers/net/can/dev.c | 39 +++++++++++
> drivers/net/can/m_can/m_can.c | 81 ++++++++++++++++++++--
> include/linux/can/dev.h | 8 +++
> 5 files changed, 156 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/can/can-transceiver.txt
>
Best Regards,
Wenyou Yang
^ permalink raw reply
* [PATCH] flex_can: Correct the checking for frame length in flexcan_start_xmit()
From: Luu An Phu @ 2018-01-02 3:44 UTC (permalink / raw)
To: wg, mkl; +Cc: linux-can, netdev, stefan-gabriel.mirea, phu.luuan
From: "phu.luuan" <phu.luuan@nxp.com>
The flexcan_start_xmit() function compares the frame length with data register
length to write frame content into data[0] and data[1] register. Data register
length is 4 bytes and frame maximum length is 8 bytes.
Fix the check that compares frame length with 3. Because the register length
is 4.
Signed-off-by: Luu An Phu <phu.luuan@nxp.com>
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0626dcf..760d2c0 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -526,7 +526,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
data = be32_to_cpup((__be32 *)&cf->data[0]);
flexcan_write(data, &priv->tx_mb->data[0]);
}
- if (cf->can_dlc > 3) {
+ if (cf->can_dlc > 4) {
data = be32_to_cpup((__be32 *)&cf->data[4]);
flexcan_write(data, &priv->tx_mb->data[1]);
}
--
1.9.1
^ permalink raw reply related
* Spende von € 3.400.000,00 EUR
From: foundation @ 2018-01-02 6:21 UTC (permalink / raw)
To: Recipients
Hallo, ich Roy Cockrum, 58, aus Knoxville, Tennessee Vereinigte Staaten von Amerika, du hast eine Wohltätigkeitsspende von € 3.400.000,00 EUR, ich gewann die America Lotterie in Amerika im Wert von $ 259,9 Millionen, und ich gebe einen Teil davon Zu fünf glücklichen Leuten und Wohltätigkeitshäusern in Gelübde, um die Armut vor dieser Welt zu beseitigen, ich kämpfe gegen die Armut. Kontaktieren Sie mich für weitere Details: roycockrumpovertyfoundation@gmail.com
^ permalink raw reply
* Re: [PATCH] flex_can: Correct the checking for frame length in flexcan_start_xmit()
From: Oliver Hartkopp @ 2018-01-02 11:17 UTC (permalink / raw)
To: Luu An Phu, wg, mkl; +Cc: linux-can, netdev, stefan-gabriel.mirea
In-Reply-To: <1514864658-13194-1-git-send-email-phu.luuan@nxp.com>
On 01/02/2018 04:44 AM, Luu An Phu wrote:
> From: "phu.luuan" <phu.luuan@nxp.com>
>
> The flexcan_start_xmit() function compares the frame length with data register
> length to write frame content into data[0] and data[1] register. Data register
> length is 4 bytes and frame maximum length is 8 bytes.
>
> Fix the check that compares frame length with 3. Because the register length
> is 4.
>
> Signed-off-by: Luu An Phu <phu.luuan@nxp.com>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Thanks for this improvement!
Best regards,
Oliver
> ---
> drivers/net/can/flexcan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 0626dcf..760d2c0 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -526,7 +526,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> data = be32_to_cpup((__be32 *)&cf->data[0]);
> flexcan_write(data, &priv->tx_mb->data[0]);
> }
> - if (cf->can_dlc > 3) {
> + if (cf->can_dlc > 4) {
> data = be32_to_cpup((__be32 *)&cf->data[4]);
> flexcan_write(data, &priv->tx_mb->data[1]);
> }
>
^ permalink raw reply
* Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate
From: Marc Kleine-Budde @ 2018-01-02 13:00 UTC (permalink / raw)
To: Faiz Abbas, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <1513949488-13026-2-git-send-email-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 5470 bytes --]
On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr <fcooper@ti.com>
>
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.
>
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> [nsekhar@ti.com: fix build error with !CONFIG_OF]
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
> v6 changes:
> fix build error with !CONFIG_OF
>
> drivers/net/can/dev.c | 39 +++++++++++++++++++++++++++++++++++++++
> include/linux/can/dev.h | 8 ++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 365a8cc..007cfc0 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -27,6 +27,7 @@
> #include <linux/can/skb.h>
> #include <linux/can/netlink.h>
> #include <linux/can/led.h>
> +#include <linux/of.h>
> #include <net/rtnetlink.h>
>
> #define MOD_DESC "CAN device driver interface"
> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
> }
> EXPORT_SYMBOL_GPL(open_candev);
>
> +#ifdef CONFIG_OF
> +/*
> + * Common function that can be used to understand the limitation of
> + * a transceiver when it provides no means to determine these limitations
> + * at runtime.
> + */
> +void of_can_transceiver(struct net_device *dev)
> +{
> + struct device_node *dn;
> + struct can_priv *priv = netdev_priv(dev);
> + struct device_node *np = dev->dev.parent->of_node;
> + int ret;
> +
> + dn = of_get_child_by_name(np, "can-transceiver");
> + if (!dn)
> + return;
> +
> + ret = of_property_read_u32(dn, "max-bitrate", &priv->max_bitrate);
> + if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
> + netdev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit.\n");
> +}
> +EXPORT_SYMBOL_GPL(of_can_transceiver);
> +#endif
> +
> /*
> * Common close function for cleanup before the device gets closed.
> *
> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> priv->bitrate_const_cnt);
> if (err)
> return err;
> +
> + if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
> + priv->max_bitrate);
> + return -EINVAL;
> + }
What about movong this check into can_get_bittiming()? Although we loose
the "arbitration" vs "canfd data".
> +
> memcpy(&priv->bittiming, &bt, sizeof(bt));
>
> if (priv->do_set_bittiming) {
> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> priv->data_bitrate_const_cnt);
> if (err)
> return err;
> +
> + if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
> + priv->max_bitrate);
> + return -EINVAL;
> + }
> +
> memcpy(&priv->data_bittiming, &dbt, sizeof(dbt));
>
> if (priv->do_set_data_bittiming) {
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 61f1cf2..fbb7810 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -48,6 +48,8 @@ struct can_priv {
> unsigned int data_bitrate_const_cnt;
> struct can_clock clock;
>
> + unsigned int max_bitrate;
Please make it an u32, name it "bitrate_max" and ...
>> struct can_priv {
>> struct net_device *dev;
>> struct can_device_stats can_stats;
>>
>> struct can_bittiming bittiming, data_bittiming;
>> const struct can_bittiming_const *bittiming_const,
>> *data_bittiming_const;
>> const u16 *termination_const;
>> unsigned int termination_const_cnt;
>> u16 termination;
>> const u32 *bitrate_const;
>> unsigned int bitrate_const_cnt;
>> const u32 *data_bitrate_const;
>> unsigned int data_bitrate_const_cnt;
...move it here.
>> struct can_clock clock;
>
> +
> enum can_state state;
>
> /* CAN controller features - see include/uapi/linux/can/netlink.h */
> @@ -166,6 +168,12 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
> void can_free_echo_skb(struct net_device *dev, unsigned int idx);
>
> +#ifdef CONFIG_OF
> +void of_can_transceiver(struct net_device *dev);
> +#else
> +static inline void of_can_transceiver(struct net_device *dev) { }
> +#endif
> +
> struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
> struct sk_buff *alloc_canfd_skb(struct net_device *dev,
> struct canfd_frame **cfd);
>
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v6 4/6] can: m_can: Support higher speed CAN-FD bitrates
From: Marc Kleine-Budde @ 2018-01-02 13:35 UTC (permalink / raw)
To: Faiz Abbas, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <1513949488-13026-5-git-send-email-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 4830 bytes --]
On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr <fcooper@ti.com>
>
> During test transmitting using CAN-FD at high bitrates (> 2 Mbps)
> would fail. Scoping the signals I noticed that only a single bit
> was being transmitted and with a bit more investigation realized the actual
> MCAN IP would go back to initialization mode automatically.
>
> It appears this issue is due to the MCAN needing to use the Transmitter
> Delay Compensation Mode with the correct value for the transmitter delay
> compensation offset (tdco). What impacts the tdco value isn't 100% clear
> but to calculate it you use an equation defined in the MCAN User's Guide.
>
> The user guide mentions that this register needs to be set based on clock
> values, secondary sample point and the data bitrate. One of the key
> variables that can't automatically be determined is the secondary
> sample point (ssp). This ssp is similar to the sp but is specific to this
> transmitter delay compensation mode. The guidelines for configuring
> ssp is rather vague but via some CAN test it appears for DRA76x that putting
> the value same as data sampling point works.
>
> The CAN-CIA's "Bit Time Requirements for CAN FD" paper presented at
> the International CAN Conference 2013 indicates that this TDC mode is
> only needed for data bit rates above 2.5 Mbps. Therefore, only enable
> this mode when the data bit rate is above 2.5 Mbps.
>
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
> drivers/net/can/m_can/m_can.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 53e764f..371ffc1 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -127,6 +127,12 @@ enum m_can_mram_cfg {
> #define DBTP_DSJW_SHIFT 0
> #define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>
> +/* Transmitter Delay Compensation Register (TDCR) */
> +#define TDCR_TDCO_SHIFT 8
> +#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
> +#define TDCR_TDCF_SHIFT 0
> +#define TDCR_TDCF_MASK (0x7F << TDCR_TDCF_SHIFT)
> +
> /* Test Register (TEST) */
> #define TEST_LBCK BIT(4)
>
> @@ -991,7 +997,8 @@ static int m_can_set_bittiming(struct net_device *dev)
> const struct can_bittiming *bt = &priv->can.bittiming;
> const struct can_bittiming *dbt = &priv->can.data_bittiming;
> u16 brp, sjw, tseg1, tseg2;
> - u32 reg_btp;
> + u32 reg_btp, tdco, ssp;
Please move the tdco and the ssp into "if (dbt->bitrate > 2500000)" scope.
Initialize "reg_btp = 0", see below.
> + bool enable_tdc = false;
Please remove, see below.
>
> brp = bt->brp - 1;
> sjw = bt->sjw - 1;
> @@ -1006,9 +1013,41 @@ static int m_can_set_bittiming(struct net_device *dev)
> sjw = dbt->sjw - 1;
> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> tseg2 = dbt->phase_seg2 - 1;
> +
> + /* TDC is only needed for bitrates beyond 2.5 MBit/s.
> + * This is mentioned in the "Bit Time Requirements for CAN FD"
> + * paper presented at the International CAN Conference 2013
> + */
> + if (dbt->bitrate > 2500000) {
> + /* Use the same value of secondary sampling point
> + * as the data sampling point
> + */
> + ssp = dbt->sample_point;
> +
> + /* Equation based on Bosch's M_CAN User Manual's
> + * Transmitter Delay Compensation Section
> + */
> + tdco = (priv->can.clock.freq / 1000) *
> + ssp / dbt->bitrate;
> +
> + /* Max valid TDCO value is 127 */
> + if (tdco > 127) {
> + netdev_warn(dev, "TDCO value of %u is beyond maximum limit. Disabling Transmitter Delay Compensation mode\n",
"maximum limit"? Either "maximum" or "limit" should be enough. If the
value is above 127, does it make sense to disable the tdco completely?
> + tdco);
> + } else {
> + enable_tdc = true;
Why not set "reg_btp |= DBTP_TDC;" here directly?
> + m_can_write(priv, M_CAN_TDCR,
> + tdco << TDCR_TDCO_SHIFT);
> + }
> + }
> +
> reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
> (tseg1 << DBTP_DTSEG1_SHIFT) |
> (tseg2 << DBTP_DTSEG2_SHIFT);
Adjust this to "reg_btp |=".
> +
> + if (enable_tdc)
> + reg_btp |= DBTP_TDC;
Please remove.
> +
> m_can_write(priv, M_CAN_DBTP, reg_btp);
> }
>
>
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v6 3/6] can: m_can: Add PM Runtime
From: Marc Kleine-Budde @ 2018-01-02 16:07 UTC (permalink / raw)
To: Faiz Abbas, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <1513949488-13026-4-git-send-email-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 4391 bytes --]
On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr <fcooper@ti.com>
>
> Add support for PM Runtime which is the new way to handle managing clocks.
> However, to avoid breaking SoCs not using PM_RUNTIME leave the old clk
> management approach in place.
There is no PM_RUNTIME anymore since 464ed18ebdb6 ("PM: Eliminate
CONFIG_PM_RUNTIME")
Have a look at the discussion: https://patchwork.kernel.org/patch/9436507/ :
>> Well, I admit it would be nicer if drivers didn't have to worry about
>> whether or not CONFIG_PM was enabled. A slightly cleaner approach
>> from the one outlined above would have the probe routine do this:
>>
>> my_power_up(dev);
>> pm_runtime_set_active(dev);
>> pm_runtime_get_noresume(dev);
>> pm_runtime_enable(dev);
> PM_RUNTIME is required by OMAP based devices to handle clock management.
> Therefore, this allows future Texas Instruments SoCs that have the MCAN IP
> to work with this driver.
Who will set the SET_RUNTIME_PM_OPS in this case?
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> [nsekhar@ti.com: handle pm_runtime_get_sync() failure, fix some bugs]
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
> drivers/net/can/m_can/m_can.c | 38 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index f72116e..53e764f 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -23,6 +23,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/iopoll.h>
> #include <linux/can/dev.h>
>
> @@ -625,19 +626,33 @@ static int m_can_clk_start(struct m_can_priv *priv)
> {
> int err;
>
> + err = pm_runtime_get_sync(priv->device);
> + if (err) {
> + pm_runtime_put_noidle(priv->device);
Why do you call this in case of an error?
> + return err;
> + }
> +
> err = clk_prepare_enable(priv->hclk);
> if (err)
> - return err;
> + goto pm_runtime_put;
>
> err = clk_prepare_enable(priv->cclk);
> if (err)
> - clk_disable_unprepare(priv->hclk);
> + goto disable_hclk;
>
> return err;
> +
> +disable_hclk:
> + clk_disable_unprepare(priv->hclk);
> +pm_runtime_put:
> + pm_runtime_put_sync(priv->device);
> + return err;
> }
>
> static void m_can_clk_stop(struct m_can_priv *priv)
> {
> + pm_runtime_put_sync(priv->device);
> +
> clk_disable_unprepare(priv->cclk);
> clk_disable_unprepare(priv->hclk);
> }
> @@ -1577,13 +1592,20 @@ static int m_can_plat_probe(struct platform_device *pdev)
> /* Enable clocks. Necessary to read Core Release in order to determine
> * M_CAN version
> */
> + pm_runtime_enable(&pdev->dev);
> + ret = pm_runtime_get_sync(&pdev->dev);
> + if (ret) {
> + pm_runtime_put_noidle(&pdev->dev);
Why do you call this in case of error?
> + goto pm_runtime_fail;
> + }
> +
> ret = clk_prepare_enable(hclk);
> if (ret)
> - goto disable_hclk_ret;
> + goto pm_runtime_put;
>
> ret = clk_prepare_enable(cclk);
> if (ret)
> - goto disable_cclk_ret;
> + goto disable_hclk_ret;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
> addr = devm_ioremap_resource(&pdev->dev, res);
> @@ -1666,6 +1688,11 @@ static int m_can_plat_probe(struct platform_device *pdev)
> clk_disable_unprepare(cclk);
> disable_hclk_ret:
> clk_disable_unprepare(hclk);
> +pm_runtime_put:
> + pm_runtime_put_sync(&pdev->dev);
> +pm_runtime_fail:
> + if (ret)
> + pm_runtime_disable(&pdev->dev);
> failed_ret:
> return ret;
> }
> @@ -1723,6 +1750,9 @@ static int m_can_plat_remove(struct platform_device *pdev)
> struct net_device *dev = platform_get_drvdata(pdev);
>
> unregister_m_can_dev(dev);
> +
> + pm_runtime_disable(&pdev->dev);
> +
> platform_set_drvdata(pdev, NULL);
>
> free_m_can_dev(dev);
>
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate
From: Marc Kleine-Budde @ 2018-01-02 16:15 UTC (permalink / raw)
To: Faiz Abbas, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <1513949488-13026-2-git-send-email-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 1179 bytes --]
On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr <fcooper@ti.com>
>
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.
Please add support to read the maximum bitrate via netlink. Have a look
at 12a6075cabc0 ("can: dev: add CAN interface termination API"). I think
you need to extend the following functions: can_get_size() and
can_fill_info().
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 00/27] kill devm_ioremap_nocache
From: Yisheng Xie @ 2018-01-03 6:42 UTC (permalink / raw)
To: christophe leroy, Guenter Roeck, Greg KH, starvik-VrBV9hrLPhE,
jesper.nilsson-VrBV9hrLPhE, tony.luck-ral2JQCrhuEAvxtiuMwx3w,
fenghua.yu-ral2JQCrhuEAvxtiuMwx3w, jonas-A9uVI2HLR7kOP4wsBPIw7w,
shorne-Re5JQEeQqe8AvxtiuMwx3w, dhowells-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, ysxie-H32Fclmsjq1BDgjK7y7TUQ,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
richard-/L3Ra7n9ekc, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
cyrille.pitchen-yU5RGvR974pGWvitb5QawA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, wim-IQzOog9fTRqzQB+pC5nmwQ,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, ralf-6z/3iImG2C8G8FEW9MqTrA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
tglx-hfZtesqFncYOwBW4kG4KsQ, jason-NLaQJdtUoK4Be96aLqz0jA,
marc.zyngier-5wv7dgnIgG8, arnd-r2nGTMty4D4,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
industrypack-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
wg-5Yr1BZd7O62+XT7JhA+gdA, mkl-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-can-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <6c0ade63-f4d3-d44d-c622-b091eb2ba902-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ cris/ia64/mn10300/openrisc maintainers
On 2017/12/25 9:09, Yisheng Xie wrote:
> hi Christophe and Greg,
>
> On 2017/12/24 16:55, christophe leroy wrote:
>>
>>
>> Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
>>> On 12/23/2017 05:48 AM, Greg KH wrote:
>>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>>> Hi all,
>>>>>
>>>>> When I tried to use devm_ioremap function and review related code, I found
>>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>>>> except one use ioremap while the other use ioremap_nocache.
>>>>
>>>> For all arches? Really? Look at MIPS, and x86, they have different
>>>> functions.
>>>>
>>>
>>> Both mips and x86 end up mapping the same function, but other arches don't.
>>> mn10300 is one where ioremap and ioremap_nocache are definitely different.
>>
>> alpha: identical
>> arc: identical
>> arm: identical
>> arm64: identical
>> cris: different <==
>> frv: identical
>> hexagone: identical
>> ia64: different <==
>> m32r: identical
>> m68k: identical
>> metag: identical
>> microblaze: identical
>> mips: identical
>> mn10300: different <==
>> nios: identical
>> openrisc: different <==
>> parisc: identical
>> riscv: identical
>> s390: identical
>> sh: identical
>> sparc: identical
>> tile: identical
>> um: rely on asm/generic
>> unicore32: identical
>> x86: identical
>> asm/generic (no mmu): identical
>
> Wow, that's correct, sorry for I have just checked the main archs, I means
> x86,arm, arm64, mips.
>
> However, I stall have no idea about why these 4 archs want different ioremap
> function with others. Drivers seems cannot aware this? If driver call ioremap
> want he really want for there 4 archs, cache or nocache?
Could you please help about this? it is out of my knowledge.
Thanks
Yisheng
>
>>
>> So 4 among all arches seems to have ioremap() and ioremap_nocache() being different.
>>
>> Could we have a define set by the 4 arches on which ioremap() and ioremap_nocache() are different, something like HAVE_DIFFERENT_IOREMAP_NOCACHE ?
>
> Then, what the HAVE_DIFFERENT_IOREMAP_NOCACHE is uesed for ?
>
> Thanks
> Yisheng
>>
>> Christophe
>>
>>>
>>> Guenter
>>>
>>>>> While ioremap's
>>>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>>>> same function with devm_ioremap, which can just be killed to reduce the size
>>>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>>>
>>>>> I have posted two versions, which use macro instead of function for
>>>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>>>> thing. So here comes v3 and please help to review.
>>>>
>>>> I don't think this can be done, what am I missing? These functions are
>>>> not identical, sorry for missing that before.
>
> Never mind, I should checked all the arches, sorry about that.
>
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> ---
>> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
>> https://www.avast.com/antivirus
>>
>>
>> .
>>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH] flex_can: Correct the checking for frame length in flexcan_start_xmit()
From: Marc Kleine-Budde @ 2018-01-03 10:35 UTC (permalink / raw)
To: Luu An Phu, wg; +Cc: linux-can, netdev, stefan-gabriel.mirea
In-Reply-To: <1514864658-13194-1-git-send-email-phu.luuan@nxp.com>
[-- Attachment #1.1: Type: text/plain, Size: 766 bytes --]
On 01/02/2018 04:44 AM, Luu An Phu wrote:
> From: "phu.luuan" <phu.luuan@nxp.com>
>
> The flexcan_start_xmit() function compares the frame length with data register
> length to write frame content into data[0] and data[1] register. Data register
> length is 4 bytes and frame maximum length is 8 bytes.
>
> Fix the check that compares frame length with 3. Because the register length
> is 4.
>
> Signed-off-by: Luu An Phu <phu.luuan@nxp.com>
Applied to can.
Tnx,
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] CAN NETWORK DRIVERS: set cf->can_id to CAN_ERR_CRTL when cf->data[1] is filled with either CAN_STATE_ERROR_WARNING or CAN_STATE_ERROR_PASSIVE
From: Marc Kleine-Budde @ 2018-01-03 10:50 UTC (permalink / raw)
To: Lederhilger Martin, wg@grandegger.com; +Cc: linux-can@vger.kernel.org
In-Reply-To: <d2b8a0f4-e274-ef88-4428-b17c8a4f1966@ds-automotion.com>
[-- Attachment #1.1: Type: text/plain, Size: 577 bytes --]
On 12/21/2017 03:42 PM, Lederhilger Martin wrote:
> This patch improves error reporting of the CAN driver for EMS Dr. Thomas Wuensche CPC-USB/ARM7.
> Signed-off-by: Martin Lederhilger <m.lederhilger@ds-automotion.com>
Added to can after rephrasing the subject and commit message.
Tnx,
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate
From: Faiz Abbas @ 2018-01-03 12:19 UTC (permalink / raw)
To: Marc Kleine-Budde, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <b3cb6058-f61f-b643-a26f-c8330d9a3ffa@pengutronix.de>
Hi Marc,
On Tuesday 02 January 2018 06:30 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> [nsekhar@ti.com: fix build error with !CONFIG_OF]
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>> ---
>> v6 changes:
>> fix build error with !CONFIG_OF
>>
>> drivers/net/can/dev.c | 39 +++++++++++++++++++++++++++++++++++++++
>> include/linux/can/dev.h | 8 ++++++++
>> 2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>> index 365a8cc..007cfc0 100644
>> --- a/drivers/net/can/dev.c
>> +++ b/drivers/net/can/dev.c
>> @@ -27,6 +27,7 @@
>> #include <linux/can/skb.h>
>> #include <linux/can/netlink.h>
>> #include <linux/can/led.h>
>> +#include <linux/of.h>
>> #include <net/rtnetlink.h>
>>
>> #define MOD_DESC "CAN device driver interface"
>> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
>> }
>> EXPORT_SYMBOL_GPL(open_candev);
>>
>> +#ifdef CONFIG_OF
>> +/*
>> + * Common function that can be used to understand the limitation of
>> + * a transceiver when it provides no means to determine these limitations
>> + * at runtime.
>> + */
>> +void of_can_transceiver(struct net_device *dev)
>> +{
>> + struct device_node *dn;
>> + struct can_priv *priv = netdev_priv(dev);
>> + struct device_node *np = dev->dev.parent->of_node;
>> + int ret;
>> +
>> + dn = of_get_child_by_name(np, "can-transceiver");
>> + if (!dn)
>> + return;
>> +
>> + ret = of_property_read_u32(dn, "max-bitrate", &priv->max_bitrate);
>> + if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
>> + netdev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit.\n");
>> +}
>> +EXPORT_SYMBOL_GPL(of_can_transceiver);
>> +#endif
>> +
>> /*
>> * Common close function for cleanup before the device gets closed.
>> *
>> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>> priv->bitrate_const_cnt);
>> if (err)
>> return err;
>> +
>> + if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
>> + netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
>> + priv->max_bitrate);
>> + return -EINVAL;
>> + }
>
> What about movong this check into can_get_bittiming()? Although we loose
> the "arbitration" vs "canfd data".
I would prefer to keep the distinction.
>
>> +
>> memcpy(&priv->bittiming, &bt, sizeof(bt));
>>
>> if (priv->do_set_bittiming) {
>> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>> priv->data_bitrate_const_cnt);
>> if (err)
>> return err;
>> +
>> + if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
>> + netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
>> + priv->max_bitrate);
>> + return -EINVAL;
>> + }
>> +
>> memcpy(&priv->data_bittiming, &dbt, sizeof(dbt));
>>
>> if (priv->do_set_data_bittiming) {
>> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
>> index 61f1cf2..fbb7810 100644
>> --- a/include/linux/can/dev.h
>> +++ b/include/linux/can/dev.h
>> @@ -48,6 +48,8 @@ struct can_priv {
>> unsigned int data_bitrate_const_cnt;
>> struct can_clock clock;
>>
>> + unsigned int max_bitrate;
>
> Please make it an u32, name it "bitrate_max" and ...
Sure.
>
>>> struct can_priv {
>>> struct net_device *dev;
>>> struct can_device_stats can_stats;
>>>
>>> struct can_bittiming bittiming, data_bittiming;
>>> const struct can_bittiming_const *bittiming_const,
>>> *data_bittiming_const;
>>> const u16 *termination_const;
>>> unsigned int termination_const_cnt;
>>> u16 termination;
>>> const u32 *bitrate_const;
>>> unsigned int bitrate_const_cnt;
>>> const u32 *data_bitrate_const;
>>> unsigned int data_bitrate_const_cnt;
>
> ...move it here.
Ok.
Thanks,
Faiz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox