Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 22:16 +0800, 王文虎 wrote:
> > On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:>> On Thu, Apr 16, 2020
> > at 11:58:29PM -0500, Scott Wood wrote:
> > > > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > > > Sounds it is. And does the modification below fit well?
> > > > > ---
> > > > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > > - { .compatible = "uio,mpc85xx-cache-sram", },
> > > > > - {},
> > > > > +#ifdef CONFIG_OF
> > > > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > > > + { /* This is filled with module_parm */ },
> > > > > + { /* Sentinel */ },
> > > > > };
> > > > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > > > +module_param_string(of_id,
> > > > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > > > + sizeof(uio_fsl_85xx_cache_sram_of_match[
> > > > > 0].c
> > > > > ompa
> > > > > tible), 0);
> > > > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > > > sram-
> > > > > uio");
> > > > > +#endif
> > > >
> > > > No. The point is that you wouldn't be configuring this with the
> > > > device
> > > > tree
> > > > at all.
> > >
> > > Wait, why not? Don't force people to use module parameters, that is
> > > crazy. DT describes the hardware involved, if someone wants to bind to
> > > a specific range of memory, as described by DT, why can't they do so?
> >
> > Yes, DT describes the hardware, and as I've said a couple times already,
> > this
> > isn't hardware description.
> >
> > I'm not forcing people to use module parameters. That was a least-effort
> > suggestion to avoid abusing the DT. I later said I'd try to come up with
> > a
> > patch that allocates regions dynamically (and most likely doesn't use UIO
> > at
> > all).
> >
> > > I can understand not liking the name "uio" in a dt tree, but there's no
> > > reason that DT can not describe what a driver binds to here.
> >
> > The DT already describes this hardware, and there is already code that
> > binds
> > to it. This patch is trying to add a second node for it with
> > configuration.
> >
>
> Hi, Scott, Greg,
> Seems like no balance here. How about I implement a driver of uio including
> the l2ctrl and cache_sram related implementations?
> And this way, the driver would be a hardware level driver and targeted for
> uio.
No, duplicating the code makes no sense whatsoever. Please just wait a bit
and I'll send a patch to have the existing driver expose a dynamic allocation
interface to userspace.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
>On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:>> On Thu, Apr 16, 2020 at
>11:58:29PM -0500, Scott Wood wrote:
>> > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
>> > > Sounds it is. And does the modification below fit well?
>> > > ---
>> > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
>> > > - { .compatible = "uio,mpc85xx-cache-sram", },
>> > > - {},
>> > > +#ifdef CONFIG_OF
>> > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
>> > > + { /* This is filled with module_parm */ },
>> > > + { /* Sentinel */ },
>> > > };
>> > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
>> > > +module_param_string(of_id,
>> > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
>> > > + sizeof(uio_fsl_85xx_cache_sram_of_match[0].c
>> > > ompa
>> > > tible), 0);
>> > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
>> > > sram-
>> > > uio");
>> > > +#endif
>> >
>> > No. The point is that you wouldn't be configuring this with the device
>> > tree
>> > at all.
>>
>> Wait, why not? Don't force people to use module parameters, that is
>> crazy. DT describes the hardware involved, if someone wants to bind to
>> a specific range of memory, as described by DT, why can't they do so?
>
>Yes, DT describes the hardware, and as I've said a couple times already, this
>isn't hardware description.
>
>I'm not forcing people to use module parameters. That was a least-effort
>suggestion to avoid abusing the DT. I later said I'd try to come up with a
>patch that allocates regions dynamically (and most likely doesn't use UIO at
>all).
>
>> I can understand not liking the name "uio" in a dt tree, but there's no
>> reason that DT can not describe what a driver binds to here.
>
>The DT already describes this hardware, and there is already code that binds
>to it. This patch is trying to add a second node for it with configuration.
>
Hi, Scott, Greg,
Seems like no balance here. How about I implement a driver of uio including
the l2ctrl and cache_sram related implementations?
And this way, the driver would be a hardware level driver and targeted for uio.
Regards,
Wenhu
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:
> On Thu, Apr 16, 2020 at 11:58:29PM -0500, Scott Wood wrote:
> > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > Sounds it is. And does the modification below fit well?
> > > ---
> > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > - { .compatible = "uio,mpc85xx-cache-sram", },
> > > - {},
> > > +#ifdef CONFIG_OF
> > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > + { /* This is filled with module_parm */ },
> > > + { /* Sentinel */ },
> > > };
> > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > +module_param_string(of_id,
> > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > + sizeof(uio_fsl_85xx_cache_sram_of_match[0].c
> > > ompa
> > > tible), 0);
> > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > sram-
> > > uio");
> > > +#endif
> >
> > No. The point is that you wouldn't be configuring this with the device
> > tree
> > at all.
>
> Wait, why not? Don't force people to use module parameters, that is
> crazy. DT describes the hardware involved, if someone wants to bind to
> a specific range of memory, as described by DT, why can't they do so?
Yes, DT describes the hardware, and as I've said a couple times already, this
isn't hardware description.
I'm not forcing people to use module parameters. That was a least-effort
suggestion to avoid abusing the DT. I later said I'd try to come up with a
patch that allocates regions dynamically (and most likely doesn't use UIO at
all).
> I can understand not liking the name "uio" in a dt tree, but there's no
> reason that DT can not describe what a driver binds to here.
The DT already describes this hardware, and there is already code that binds
to it. This patch is trying to add a second node for it with configuration.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Thu, Apr 16, 2020 at 11:58:29PM -0500, Scott Wood wrote:
> On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > > On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> > > > > +#define UIO_INFO_VER "devicetree,pseudo"
> > > >
> > > > What does this mean? Changing a number into a non-obvious string (Why
> > > > "pseudo"? Why does the UIO user care that the config came from the
> > > > device
> > > > tree?) just to avoid setting off Greg's version number autoresponse
> > > > isn't
> > > > really helping anything.
> > > >
> > > > > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > > + { .compatible = "uio,mpc85xx-cache-sram", },
> > >
> > > Form is , and "uio" is not a vendor (and never will be).
> > >
> >
> > Should have been something like "fsl,mpc85xx-cache-sram-uio", and if it is
> > to be defined with module parameters, this would be user defined.
> > Anyway, , should always be used.
> >
> > > > > + {},
> > > > > +};
> > > > > +
> > > > > +static struct platform_driver uio_fsl_85xx_cache_sram = {
> > > > > + .probe = uio_fsl_85xx_cache_sram_probe,
> > > > > + .remove = uio_fsl_85xx_cache_sram_remove,
> > > > > + .driver = {
> > > > > + .name = DRIVER_NAME,
> > > > > + .owner = THIS_MODULE,
> > > > > + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
> > > > > + },
> > > > > +};
> > > >
> > > > Greg's comment notwithstanding, I really don't think this belongs in the
> > > > device tree (and if I do get overruled on that point, it at least needs
> > > > a
> > > > binding document). Let me try to come up with a patch for dynamic
> > > > allocation.
> > >
> > > Agreed. "UIO" bindings have long been rejected.
> > >
> >
> > Sounds it is. And does the modification below fit well?
> > ---
> > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > - { .compatible = "uio,mpc85xx-cache-sram", },
> > - {},
> > +#ifdef CONFIG_OF
> > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > + { /* This is filled with module_parm */ },
> > + { /* Sentinel */ },
> > };
> > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > +module_param_string(of_id, uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > + sizeof(uio_fsl_85xx_cache_sram_of_match[0].compa
> > tible), 0);
> > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-sram-
> > uio");
> > +#endif
>
> No. The point is that you wouldn't be configuring this with the device tree
> at all.
Wait, why not? Don't force people to use module parameters, that is
crazy. DT describes the hardware involved, if someone wants to bind to
a specific range of memory, as described by DT, why can't they do so?
I can understand not liking the name "uio" in a dt tree, but there's no
reason that DT can not describe what a driver binds to here.
Remember, module parameters are NEVER the answer, this isn't the 1990's
anymore.
thanks,
greg k-h
Re:Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
>> > > On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
>> > > > +#define UIO_INFO_VER "devicetree,pseudo"
>> > >
>> > > What does this mean? Changing a number into a non-obvious string (Why
>> > > "pseudo"? Why does the UIO user care that the config came from the
>> > > device
>> > > tree?) just to avoid setting off Greg's version number autoresponse
>> > > isn't
>> > > really helping anything.
As I mentioned before, this is not currently meaningful for us, and maybe the
better
way is to set it optionally for uio, but it belongs to uio core, which is a
framework for
different kind of drivers or devices, but not only for us. So I guess this is
not a thing
troubles and arguing about this is really helpless.
>> > >
>> > > > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
>> > > > + { .compatible = "uio,mpc85xx-cache-sram", },
>> >
>> > Form is , and "uio" is not a vendor (and never will be).
>> >
>>
>> Should have been something like "fsl,mpc85xx-cache-sram-uio", and if it is
>> to be defined with module parameters, this would be user defined.
>> Anyway, , should always be used.
>>
>> > > > + {},
>> > > > +};
>> > > > +
>> > > > +static struct platform_driver uio_fsl_85xx_cache_sram = {
>> > > > + .probe = uio_fsl_85xx_cache_sram_probe,
>> > > > + .remove = uio_fsl_85xx_cache_sram_remove,
>> > > > + .driver = {
>> > > > + .name = DRIVER_NAME,
>> > > > + .owner = THIS_MODULE,
>> > > > + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
>> > > > + },
>> > > > +};
>> > >
>> > > Greg's comment notwithstanding, I really don't think this belongs in the
>> > > device tree (and if I do get overruled on that point, it at least needs
>> > > a
>> > > binding document). Let me try to come up with a patch for dynamic
>> > > allocation.
>> >
>> > Agreed. "UIO" bindings have long been rejected.
>> >
>>
>> Sounds it is. And does the modification below fit well?
>> ---
>> -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
>> - { .compatible = "uio,mpc85xx-cache-sram", },
>> - {},
>> +#ifdef CONFIG_OF
>> +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
>> + { /* This is filled with module_parm */ },
>> + { /* Sentinel */ },
>> };
>> +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
>> +module_param_string(of_id, uio_fsl_85xx_cache_sram_of_match[0].compatible,
>> + sizeof(uio_fsl_85xx_cache_sram_of_match[0].compa
>> tible), 0);
>> +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-sram-
>> uio");
>> +#endif
>
>No. The point is that you wouldn't be configuring this with the device tree
>at all.
>
It was to fit for the unbinding requriement. And I mentioned what if I want to
create
more than one device and each owns multiple uiomaps?
Devicetree is definitely better choice to solve the problem and make the driver
more
convenient for users to use. Pseudo device is a device and a device. Or else
device
tree should be hardware-only-devicetree.
The point is why we left the better choice and write a plenty of redundant codes
to parse the module parameters?
Further more, I don't think there is enough reason for the lower driver mpc85xx
cache-sram
to restrict other from using it in a way of module param or dynamic allocation
with ioctl or so.
UIO is there and all these parts cooperate well to make the cache-sram more
useful and better
resolve user requirement.
I will update the patch with the diff applied in v5.
Thanks,
Wenhu
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> > > > +#define UIO_INFO_VER "devicetree,pseudo"
> > >
> > > What does this mean? Changing a number into a non-obvious string (Why
> > > "pseudo"? Why does the UIO user care that the config came from the
> > > device
> > > tree?) just to avoid setting off Greg's version number autoresponse
> > > isn't
> > > really helping anything.
> > >
> > > > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > + { .compatible = "uio,mpc85xx-cache-sram", },
> >
> > Form is , and "uio" is not a vendor (and never will be).
> >
>
> Should have been something like "fsl,mpc85xx-cache-sram-uio", and if it is
> to be defined with module parameters, this would be user defined.
> Anyway, , should always be used.
>
> > > > + {},
> > > > +};
> > > > +
> > > > +static struct platform_driver uio_fsl_85xx_cache_sram = {
> > > > + .probe = uio_fsl_85xx_cache_sram_probe,
> > > > + .remove = uio_fsl_85xx_cache_sram_remove,
> > > > + .driver = {
> > > > + .name = DRIVER_NAME,
> > > > + .owner = THIS_MODULE,
> > > > + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
> > > > + },
> > > > +};
> > >
> > > Greg's comment notwithstanding, I really don't think this belongs in the
> > > device tree (and if I do get overruled on that point, it at least needs
> > > a
> > > binding document). Let me try to come up with a patch for dynamic
> > > allocation.
> >
> > Agreed. "UIO" bindings have long been rejected.
> >
>
> Sounds it is. And does the modification below fit well?
> ---
> -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> - { .compatible = "uio,mpc85xx-cache-sram", },
> - {},
> +#ifdef CONFIG_OF
> +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> + { /* This is filled with module_parm */ },
> + { /* Sentinel */ },
> };
> +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> +module_param_string(of_id, uio_fsl_85xx_cache_sram_of_match[0].compatible,
> + sizeof(uio_fsl_85xx_cache_sram_of_match[0].compa
> tible), 0);
> +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-sram-
> uio");
> +#endif
No. The point is that you wouldn't be configuring this with the device tree
at all.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
>> On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
>> > +#define UIO_INFO_VER "devicetree,pseudo"
>>
>> What does this mean? Changing a number into a non-obvious string (Why
>> "pseudo"? Why does the UIO user care that the config came from the device
>> tree?) just to avoid setting off Greg's version number autoresponse isn't
>> really helping anything.
>>
>> > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
>> > + { .compatible = "uio,mpc85xx-cache-sram", },
>
>Form is , and "uio" is not a vendor (and never will be).
>
Should have been something like "fsl,mpc85xx-cache-sram-uio", and if it is
to be defined with module parameters, this would be user defined.
Anyway, , should always be used.
>> > + {},
>> > +};
>> > +
>> > +static struct platform_driver uio_fsl_85xx_cache_sram = {
>> > + .probe = uio_fsl_85xx_cache_sram_probe,
>> > + .remove = uio_fsl_85xx_cache_sram_remove,
>> > + .driver = {
>> > + .name = DRIVER_NAME,
>> > + .owner = THIS_MODULE,
>> > + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
>> > + },
>> > +};
>>
>> Greg's comment notwithstanding, I really don't think this belongs in the
>> device tree (and if I do get overruled on that point, it at least needs a
>> binding document). Let me try to come up with a patch for dynamic
>> allocation.
>
>Agreed. "UIO" bindings have long been rejected.
>
Sounds it is. And does the modification below fit well?
---
-static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
- { .compatible = "uio,mpc85xx-cache-sram", },
- {},
+#ifdef CONFIG_OF
+static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
+ { /* This is filled with module_parm */ },
+ { /* Sentinel */ },
};
+MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
+module_param_string(of_id, uio_fsl_85xx_cache_sram_of_match[0].compatible,
+
sizeof(uio_fsl_85xx_cache_sram_of_match[0].compatible), 0);
+MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-sram-uio");
+#endif
static struct platform_driver uio_fsl_85xx_cache_sram = {
.probe = uio_fsl_85xx_cache_sram_probe,
.remove = uio_fsl_85xx_cache_sram_remove,
.driver = {
.name = DRIVER_NAME,
- .owner = THIS_MODULE,
- .of_match_table = uio_mpc85xx_l2ctlr_of_match,
+ .of_match_table =
of_match_ptr(uio_fsl_85xx_cache_sram_of_match),
},
};
Regards,
Wenhu
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Thu, Apr 16, 2020 at 02:59:36PM -0500, Scott Wood wrote:
> On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> > +#define UIO_INFO_VER "devicetree,pseudo"
>
> What does this mean? Changing a number into a non-obvious string (Why
> "pseudo"? Why does the UIO user care that the config came from the device
> tree?) just to avoid setting off Greg's version number autoresponse isn't
> really helping anything.
>
> > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > + { .compatible = "uio,mpc85xx-cache-sram", },
Form is , and "uio" is not a vendor (and never will be).
> > + {},
> > +};
> > +
> > +static struct platform_driver uio_fsl_85xx_cache_sram = {
> > + .probe = uio_fsl_85xx_cache_sram_probe,
> > + .remove = uio_fsl_85xx_cache_sram_remove,
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .owner = THIS_MODULE,
> > + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
> > + },
> > +};
>
> Greg's comment notwithstanding, I really don't think this belongs in the
> device tree (and if I do get overruled on that point, it at least needs a
> binding document). Let me try to come up with a patch for dynamic allocation.
Agreed. "UIO" bindings have long been rejected.
Rob
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> +#define UIO_INFO_VER "devicetree,pseudo"
What does this mean? Changing a number into a non-obvious string (Why
"pseudo"? Why does the UIO user care that the config came from the device
tree?) just to avoid setting off Greg's version number autoresponse isn't
really helping anything.
> +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> + { .compatible = "uio,mpc85xx-cache-sram", },
> + {},
> +};
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> + .probe = uio_fsl_85xx_cache_sram_probe,
> + .remove = uio_fsl_85xx_cache_sram_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
> + },
> +};
Greg's comment notwithstanding, I really don't think this belongs in the
device tree (and if I do get overruled on that point, it at least needs a
binding document). Let me try to come up with a patch for dynamic allocation.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
Le 16/04/2020 à 17:35, Wang Wenhu a écrit :
A driver for freescale 85xx platforms to access the Cache-Sram form
user level. This is extremely helpful for some user-space applications
that require high performance memory accesses.
Cc: Greg Kroah-Hartman
Cc: Christophe Leroy
Cc: Scott Wood
Cc: Michael Ellerman
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Wang Wenhu
Reviewed-by: Christophe Leroy
---
Changes since v1:
* Addressed comments from Greg K-H
* Moved kfree(info->name) into uio_info_free_internal()
Changes since v2:
* Addressed comments from Greg, Scott and Christophe
* Use "uiomem->internal_addr" as if condition for sram memory free,
and memset the uiomem entry
* of_match_table modified to be apart from HW info which belong to
the HW level driver fsl_85xx_cache_sram to match
* Use roundup_pow_of_two for align calc(really learned a lot from Christophe)
* Remove useless clear block of uiomem entries.
* Use UIO_INFO_VER micro for info->version, and define it as
"devicetree,pseudo", meaning this is pseudo device and probed from
device tree configuration
Changes since v3:
* Addressed comments from Christophe(use devm_xxx memory alloc interfaces)
---
drivers/uio/Kconfig | 9 ++
drivers/uio/Makefile | 1 +
drivers/uio/uio_fsl_85xx_cache_sram.c | 148 ++
3 files changed, 158 insertions(+)
create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 202ee81cfc2b..9c3b47461b71 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,15 @@ config UIO_NETX
To compile this driver as a module, choose M here; the module
will be called uio_netx.
+config UIO_FSL_85XX_CACHE_SRAM
+ tristate "Freescale 85xx Cache-Sram driver"
+ depends on FSL_SOC_BOOKE && PPC32
+ select FSL_85XX_CACHE_SRAM
+ help
+ Generic driver for accessing the Cache-Sram form user level. This
+ is extremely helpful for some user-space applications that require
+ high performance memory accesses.
+
config UIO_FSL_ELBC_GPCM
tristate "eLBC/GPCM driver"
depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index c285dd2a4539..be2056cffc21 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -10,4 +10,5 @@ obj-$(CONFIG_UIO_NETX)+= uio_netx.o
obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
obj-$(CONFIG_UIO_MF624) += uio_mf624.o
obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM) += uio_fsl_85xx_cache_sram.o
obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c
b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index ..cb339d1f9019
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
+ * Copyright (C) 2020 Wang Wenhu
+ * All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME"uio_fsl_85xx_cache_sram"
+#define UIO_INFO_VER "devicetree,pseudo"
+#define UIO_NAME "uio_cache_sram"
+
+static void uio_info_free_internal(struct uio_info *info)
+{
+ int i;
+
+ for (i = 0; i < MAX_UIO_MAPS; i++) {
+ struct uio_mem *uiomem = >mem[i];
+
+ if (uiomem->internal_addr) {
+ mpc85xx_cache_sram_free(uiomem->internal_addr);
+ memset(uiomem, 0, sizeof(*uiomem));
+ }
+ }
+}
+
+static int uio_fsl_85xx_cache_sram_probe(struct platform_device *pdev)
+{
+ struct device_node *parent = pdev->dev.of_node;
+ struct device_node *node = NULL;
+ struct uio_info *info;
+ struct uio_mem *uiomem;
+ const char *dt_name;
+ u32 mem_size;
+ int ret;
+
+ /* alloc uio_info for one device */
+ info = devm_kzalloc(>dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ /* get optional uio name */
+ if (of_property_read_string(parent, "uio_name", _name))
+ dt_name = UIO_NAME;
+
+ info->name = devm_kstrdup(>dev, dt_name, GFP_KERNEL);
+ if (!info->name)
+ return -ENOMEM;
+
+ uiomem = info->mem;
+ for_each_child_of_node(parent, node) {
+ void *virt;
+ phys_addr_t phys;
+
+ ret = of_property_read_u32(node, "cache-mem-size", _size);
+ if (ret) {
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if (mem_size == 0) {
+ dev_err(>dev, "error cache-mem-size should not be
0\n");
+ ret = -EINVAL;
+ goto err_out;
+
[PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
A driver for freescale 85xx platforms to access the Cache-Sram form
user level. This is extremely helpful for some user-space applications
that require high performance memory accesses.
Cc: Greg Kroah-Hartman
Cc: Christophe Leroy
Cc: Scott Wood
Cc: Michael Ellerman
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Wang Wenhu
---
Changes since v1:
* Addressed comments from Greg K-H
* Moved kfree(info->name) into uio_info_free_internal()
Changes since v2:
* Addressed comments from Greg, Scott and Christophe
* Use "uiomem->internal_addr" as if condition for sram memory free,
and memset the uiomem entry
* of_match_table modified to be apart from HW info which belong to
the HW level driver fsl_85xx_cache_sram to match
* Use roundup_pow_of_two for align calc(really learned a lot from Christophe)
* Remove useless clear block of uiomem entries.
* Use UIO_INFO_VER micro for info->version, and define it as
"devicetree,pseudo", meaning this is pseudo device and probed from
device tree configuration
Changes since v3:
* Addressed comments from Christophe(use devm_xxx memory alloc interfaces)
---
drivers/uio/Kconfig | 9 ++
drivers/uio/Makefile | 1 +
drivers/uio/uio_fsl_85xx_cache_sram.c | 148 ++
3 files changed, 158 insertions(+)
create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 202ee81cfc2b..9c3b47461b71 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,15 @@ config UIO_NETX
To compile this driver as a module, choose M here; the module
will be called uio_netx.
+config UIO_FSL_85XX_CACHE_SRAM
+ tristate "Freescale 85xx Cache-Sram driver"
+ depends on FSL_SOC_BOOKE && PPC32
+ select FSL_85XX_CACHE_SRAM
+ help
+ Generic driver for accessing the Cache-Sram form user level. This
+ is extremely helpful for some user-space applications that require
+ high performance memory accesses.
+
config UIO_FSL_ELBC_GPCM
tristate "eLBC/GPCM driver"
depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index c285dd2a4539..be2056cffc21 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -10,4 +10,5 @@ obj-$(CONFIG_UIO_NETX)+= uio_netx.o
obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
obj-$(CONFIG_UIO_MF624) += uio_mf624.o
obj-$(CONFIG_UIO_FSL_ELBC_GPCM)+= uio_fsl_elbc_gpcm.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM) += uio_fsl_85xx_cache_sram.o
obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c
b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index ..cb339d1f9019
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
+ * Copyright (C) 2020 Wang Wenhu
+ * All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME"uio_fsl_85xx_cache_sram"
+#define UIO_INFO_VER "devicetree,pseudo"
+#define UIO_NAME "uio_cache_sram"
+
+static void uio_info_free_internal(struct uio_info *info)
+{
+ int i;
+
+ for (i = 0; i < MAX_UIO_MAPS; i++) {
+ struct uio_mem *uiomem = >mem[i];
+
+ if (uiomem->internal_addr) {
+ mpc85xx_cache_sram_free(uiomem->internal_addr);
+ memset(uiomem, 0, sizeof(*uiomem));
+ }
+ }
+}
+
+static int uio_fsl_85xx_cache_sram_probe(struct platform_device *pdev)
+{
+ struct device_node *parent = pdev->dev.of_node;
+ struct device_node *node = NULL;
+ struct uio_info *info;
+ struct uio_mem *uiomem;
+ const char *dt_name;
+ u32 mem_size;
+ int ret;
+
+ /* alloc uio_info for one device */
+ info = devm_kzalloc(>dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ /* get optional uio name */
+ if (of_property_read_string(parent, "uio_name", _name))
+ dt_name = UIO_NAME;
+
+ info->name = devm_kstrdup(>dev, dt_name, GFP_KERNEL);
+ if (!info->name)
+ return -ENOMEM;
+
+ uiomem = info->mem;
+ for_each_child_of_node(parent, node) {
+ void *virt;
+ phys_addr_t phys;
+
+ ret = of_property_read_u32(node, "cache-mem-size", _size);
+ if (ret) {
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if (mem_size == 0) {
+ dev_err(>dev, "error cache-mem-size should not be
0\n");
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ virt = mpc85xx_cache_sram_alloc(mem_size, ,
+
