the cost of EXPORT_SYMBOL_GPL

the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 1890 bytes --]

Hi
I am in the middle of implementation of various external kernel modules.
All kernel modules touch some board specific registers. For example let's
say we have 3 groups of register areas:
#define REG1_ADDR (0xFDEA7000)
#define REG1 _SIZE (0x3C0)
#define REG2_ADDR (0xFDEC4000)
#define REG2 _SIZE (0xFF8)
#define REG3_ADDR (0xFDEA6000)
#define REG3_SIZE (0x80)
And let's say we have 5 kernel modules:
A
B
C
D
E

Kernel module A touches only one 32-bit register X from REG1 space and a
lot of registers from REG2 space.
Kernel modules B,C,D touches various registers from REG1 and REG2 space but
not this one which kernel module A touches.
Kernel module E is the only one who touches registers from REG3 space. No
other kernel module touches REG3 space.

If I would implement the code in C++ in user space I would create dynamic
library for access to REG1,REG2,REG3 register space. Each 32-bit register
would have his own separate function to read,write to register. And then
each user space process A-E would be linked against this library during
compilation.
When programming in kernel I don't know what I should do.
Should I implement 6th external kernel module which implements all
operations on access to registers REG1,REG2,REG3??? There will be a lot of
EXPORT_SYMBOL_GPL. Or maybe this approach is NOK, I mean it is good but I
should only place in 6th helper kernel module functions which are used by
more than one kernel module. So for example functions for getting access to
REG3 address space or reg X from REG1 space should be not there. They
should be only in kernel module code A/E. Currently this is code which I
implemented. In 6th helper kernel module there are only registers which are
used by more than one kernel module. But I am not sure if it is good
approach. Maybe there should be there all registers access?

Could anybody help how to organize code here.

BR
Tomek

[-- Attachment #1.2: Type: text/html, Size: 2228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Greg KH]

On Tue, Jun 16, 2020 at 03:18:02PM +0200, Tomek The Messenger wrote:
> Hi
> I am in the middle of implementation of various external kernel modules.
> All kernel modules touch some board specific registers. For example let's
> say we have 3 groups of register areas:
> #define REG1_ADDR (0xFDEA7000)
> #define REG1 _SIZE (0x3C0)
> #define REG2_ADDR (0xFDEC4000)
> #define REG2 _SIZE (0xFF8)
> #define REG3_ADDR (0xFDEA6000)
> #define REG3_SIZE (0x80)
> And let's say we have 5 kernel modules:
> A
> B
> C
> D
> E
> 
> Kernel module A touches only one 32-bit register X from REG1 space and a
> lot of registers from REG2 space.
> Kernel modules B,C,D touches various registers from REG1 and REG2 space but
> not this one which kernel module A touches.
> Kernel module E is the only one who touches registers from REG3 space. No
> other kernel module touches REG3 space.
> 
> If I would implement the code in C++ in user space I would create dynamic
> library for access to REG1,REG2,REG3 register space. Each 32-bit register
> would have his own separate function to read,write to register. And then
> each user space process A-E would be linked against this library during
> compilation.
> When programming in kernel I don't know what I should do.
> Should I implement 6th external kernel module which implements all
> operations on access to registers REG1,REG2,REG3??? There will be a lot of
> EXPORT_SYMBOL_GPL. Or maybe this approach is NOK, I mean it is good but I
> should only place in 6th helper kernel module functions which are used by
> more than one kernel module. So for example functions for getting access to
> REG3 address space or reg X from REG1 space should be not there. They
> should be only in kernel module code A/E. Currently this is code which I
> implemented. In 6th helper kernel module there are only registers which are
> used by more than one kernel module. But I am not sure if it is good
> approach. Maybe there should be there all registers access?
> 
> Could anybody help how to organize code here.

Why is this so many different modules?  Why not just make it one?

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 3543 bytes --]

Hi
Thanks for reply.
We make some proxy layer in linux /sys. So for example we have directories:
/sys/resets
/sys/routers
etc.
So if user wants to get to know what is the reset reason he doesn't use
devmem 0xfff...., he just read the file in /sys/resets/..
If user wants to know some routing path he doesn't read registers via
devmem, he only reads some file in /sys/routers which gives him output.
So this is in order to facilitate reading/writing to registers. Instead of
searching in documentation concrete registers users have some helper proxy
layer in sysfs. And for each directory in /sys there is separate kernel
module. Division is based on some functionalities like resets, routing etc.
And the problem is should this 6th kernel module perform role of getting
access to all registers or only to parts which are used by more than one
kernel module? Now I am using second approach but when I am looking at the
code it looks terrible. For example #define for register X and function to
access it, is in module A, for egister Y in module B, for register Z in 6th
kernel module. There is mess I think and maybe better approach is to put
all API to 6th kernel module?

wt., 16.06.2020, 15:27 użytkownik Greg KH <greg@kroah.com> napisał:

> On Tue, Jun 16, 2020 at 03:18:02PM +0200, Tomek The Messenger wrote:
> > Hi
> > I am in the middle of implementation of various external kernel modules.
> > All kernel modules touch some board specific registers. For example let's
> > say we have 3 groups of register areas:
> > #define REG1_ADDR (0xFDEA7000)
> > #define REG1 _SIZE (0x3C0)
> > #define REG2_ADDR (0xFDEC4000)
> > #define REG2 _SIZE (0xFF8)
> > #define REG3_ADDR (0xFDEA6000)
> > #define REG3_SIZE (0x80)
> > And let's say we have 5 kernel modules:
> > A
> > B
> > C
> > D
> > E
> >
> > Kernel module A touches only one 32-bit register X from REG1 space and a
> > lot of registers from REG2 space.
> > Kernel modules B,C,D touches various registers from REG1 and REG2 space
> but
> > not this one which kernel module A touches.
> > Kernel module E is the only one who touches registers from REG3 space. No
> > other kernel module touches REG3 space.
> >
> > If I would implement the code in C++ in user space I would create dynamic
> > library for access to REG1,REG2,REG3 register space. Each 32-bit register
> > would have his own separate function to read,write to register. And then
> > each user space process A-E would be linked against this library during
> > compilation.
> > When programming in kernel I don't know what I should do.
> > Should I implement 6th external kernel module which implements all
> > operations on access to registers REG1,REG2,REG3??? There will be a lot
> of
> > EXPORT_SYMBOL_GPL. Or maybe this approach is NOK, I mean it is good but I
> > should only place in 6th helper kernel module functions which are used by
> > more than one kernel module. So for example functions for getting access
> to
> > REG3 address space or reg X from REG1 space should be not there. They
> > should be only in kernel module code A/E. Currently this is code which I
> > implemented. In 6th helper kernel module there are only registers which
> are
> > used by more than one kernel module. But I am not sure if it is good
> > approach. Maybe there should be there all registers access?
> >
> > Could anybody help how to organize code here.
>
> Why is this so many different modules?  Why not just make it one?
>
> thanks,
>
> greg k-h
>

[-- Attachment #1.2: Type: text/html, Size: 4159 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Greg KH]

On Wed, Jun 17, 2020 at 09:58:03AM +0200, Tomek The Messenger wrote:
> Hi
> Thanks for reply.
> We make some proxy layer in linux /sys. So for example we have directories:
> /sys/resets
> /sys/routers

Really?  That's a total abuse of sysfs, don't do that.

Seriously, that's not ok at all.

> etc.
> So if user wants to get to know what is the reset reason he doesn't use
> devmem 0xfff...., he just read the file in /sys/resets/..
> If user wants to know some routing path he doesn't read registers via
> devmem, he only reads some file in /sys/routers which gives him output.
> So this is in order to facilitate reading/writing to registers. Instead of
> searching in documentation concrete registers users have some helper proxy
> layer in sysfs. And for each directory in /sys there is separate kernel
> module. Division is based on some functionalities like resets, routing etc.
> And the problem is should this 6th kernel module perform role of getting
> access to all registers or only to parts which are used by more than one
> kernel module? Now I am using second approach but when I am looking at the
> code it looks terrible. For example #define for register X and function to
> access it, is in module A, for egister Y in module B, for register Z in 6th
> kernel module. There is mess I think and maybe better approach is to put
> all API to 6th kernel module?

I really do not understand at all, sorry.  Why are you using sysfs for
something that it is not designed for?  And sysfs directory structure
has nothing to do with kernel module names/structures.

Do you have a pointer to your code somewhere so that we can review it
and suggest the correct apis you should be using instead?

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 3437 bytes --]

OK, so I had to wrongly described something because in my company this is
popular approach to use /sys as some abstract layer to get access to
registers.
So maybe in other words.
Let's say You have many socs. In one soc reset_reason can be gotten from
devmem 0xfff.... In other soc reset_reason can be gotten through i2c read
from some device.
Apart from reset_reason You have many other functionalities like kernel
boot source (tftp, spi).
And let's assume you are tester. It is hard for those people to remember
addresses, remembering and checking all the time how to read reset_reason
or routing, then extract value from proper bits. Instead of raw addresses
they have interface in /sys in the form of files. For example they read
cat /sys/resetreg/reason
as output they got in string format for example COLD_REBOOT.
They read:
/sys/bootreg/source
As output they got in string format TFTP.
And that is on each soc. This is good that registers, the way of access is
hidden under interface. It is easier and faster to debug anything with such
approach even for developers not only testers.
And there is separate kernel module which implements read/write access to
each of directory in /sys. So we use here linux API:
kobj = kobject_create_and_add("bootreg", NULL);
sysfs_create_groups(kobj, ...)
So hope that You have now good understanding.
I cannot show the code as I don't if I am allowed. I am about half an year
in kernel development team, so sometimes I have some doubts. And now my
doubts are where to put API for read/write operations as I mentioned above.

On Wed, Jun 17, 2020 at 10:55 AM Greg KH <greg@kroah.com> wrote:

> On Wed, Jun 17, 2020 at 09:58:03AM +0200, Tomek The Messenger wrote:
> > Hi
> > Thanks for reply.
> > We make some proxy layer in linux /sys. So for example we have
> directories:
> > /sys/resets
> > /sys/routers
>
> Really?  That's a total abuse of sysfs, don't do that.
>
> Seriously, that's not ok at all.
>
> > etc.
> > So if user wants to get to know what is the reset reason he doesn't use
> > devmem 0xfff...., he just read the file in /sys/resets/..
> > If user wants to know some routing path he doesn't read registers via
> > devmem, he only reads some file in /sys/routers which gives him output.
> > So this is in order to facilitate reading/writing to registers. Instead
> of
> > searching in documentation concrete registers users have some helper
> proxy
> > layer in sysfs. And for each directory in /sys there is separate kernel
> > module. Division is based on some functionalities like resets, routing
> etc.
> > And the problem is should this 6th kernel module perform role of getting
> > access to all registers or only to parts which are used by more than one
> > kernel module? Now I am using second approach but when I am looking at
> the
> > code it looks terrible. For example #define for register X and function
> to
> > access it, is in module A, for egister Y in module B, for register Z in
> 6th
> > kernel module. There is mess I think and maybe better approach is to put
> > all API to 6th kernel module?
>
> I really do not understand at all, sorry.  Why are you using sysfs for
> something that it is not designed for?  And sysfs directory structure
> has nothing to do with kernel module names/structures.
>
> Do you have a pointer to your code somewhere so that we can review it
> and suggest the correct apis you should be using instead?
>
> thanks,
>
> greg k-h
>

[-- Attachment #1.2: Type: text/html, Size: 4079 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 4398 bytes --]

Maybe some pseudo-code:
So in other to implement file /sys/resetreg/reason You have to write:

static ssize_t reason_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
{
u32 val;

val = some_readAPI_to_get_reboot_reason();

return scnprintf(buf, PAGE_SIZE, "%d\n", (u32)(val));
}

static struct kobj_attribute reason = __ATTR_RW(reason);

static struct attribute *reason_attrs[] = {
&reason_attribute.attr,

NULL,
};
ATTRIBUTE_GROUPS(some_group);

Then this some_group symbol goes as second parameter to sysfs_create_groups.
So functions *_show go to separate kernel modules. But function
some_readAPI_to_get_reboot_reason() I think should be placed in 6th kernel
module, so in kernel module with read/write specific operations to given
soc.

On Wed, Jun 17, 2020 at 11:17 AM Tomek The Messenger <
tomekthemessenger@gmail.com> wrote:

> OK, so I had to wrongly described something because in my company this is
> popular approach to use /sys as some abstract layer to get access to
> registers.
> So maybe in other words.
> Let's say You have many socs. In one soc reset_reason can be gotten from
> devmem 0xfff.... In other soc reset_reason can be gotten through i2c read
> from some device.
> Apart from reset_reason You have many other functionalities like kernel
> boot source (tftp, spi).
> And let's assume you are tester. It is hard for those people to remember
> addresses, remembering and checking all the time how to read reset_reason
> or routing, then extract value from proper bits. Instead of raw addresses
> they have interface in /sys in the form of files. For example they read
> cat /sys/resetreg/reason
> as output they got in string format for example COLD_REBOOT.
> They read:
> /sys/bootreg/source
> As output they got in string format TFTP.
> And that is on each soc. This is good that registers, the way of access is
> hidden under interface. It is easier and faster to debug anything with such
> approach even for developers not only testers.
> And there is separate kernel module which implements read/write access to
> each of directory in /sys. So we use here linux API:
> kobj = kobject_create_and_add("bootreg", NULL);
> sysfs_create_groups(kobj, ...)
> So hope that You have now good understanding.
> I cannot show the code as I don't if I am allowed. I am about half an year
> in kernel development team, so sometimes I have some doubts. And now my
> doubts are where to put API for read/write operations as I mentioned above.
>
> On Wed, Jun 17, 2020 at 10:55 AM Greg KH <greg@kroah.com> wrote:
>
>> On Wed, Jun 17, 2020 at 09:58:03AM +0200, Tomek The Messenger wrote:
>> > Hi
>> > Thanks for reply.
>> > We make some proxy layer in linux /sys. So for example we have
>> directories:
>> > /sys/resets
>> > /sys/routers
>>
>> Really?  That's a total abuse of sysfs, don't do that.
>>
>> Seriously, that's not ok at all.
>>
>> > etc.
>> > So if user wants to get to know what is the reset reason he doesn't use
>> > devmem 0xfff...., he just read the file in /sys/resets/..
>> > If user wants to know some routing path he doesn't read registers via
>> > devmem, he only reads some file in /sys/routers which gives him output.
>> > So this is in order to facilitate reading/writing to registers. Instead
>> of
>> > searching in documentation concrete registers users have some helper
>> proxy
>> > layer in sysfs. And for each directory in /sys there is separate kernel
>> > module. Division is based on some functionalities like resets, routing
>> etc.
>> > And the problem is should this 6th kernel module perform role of getting
>> > access to all registers or only to parts which are used by more than one
>> > kernel module? Now I am using second approach but when I am looking at
>> the
>> > code it looks terrible. For example #define for register X and function
>> to
>> > access it, is in module A, for egister Y in module B, for register Z in
>> 6th
>> > kernel module. There is mess I think and maybe better approach is to put
>> > all API to 6th kernel module?
>>
>> I really do not understand at all, sorry.  Why are you using sysfs for
>> something that it is not designed for?  And sysfs directory structure
>> has nothing to do with kernel module names/structures.
>>
>> Do you have a pointer to your code somewhere so that we can review it
>> and suggest the correct apis you should be using instead?
>>
>> thanks,
>>
>> greg k-h
>>
>

[-- Attachment #1.2: Type: text/html, Size: 5393 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Greg KH]

On Wed, Jun 17, 2020 at 11:17:28AM +0200, Tomek The Messenger wrote:
> OK, so I had to wrongly described something because in my company this is
> popular approach to use /sys as some abstract layer to get access to
> registers.

Please do not do that.  There are valid kernel apis to grant access to
registers easily, and creating random files and directories in the root
of sysfs is not only the incorrect way of doing it, you are making more
work for yourself to do so, adding more code that is not needed.

Do you have a pointer to the code so that we can provide help with
showing the "correct" way to do all of this?

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 808 bytes --]

OK, I will ask if it is possible to share the code.

Thanks
BR
Tomek

On Wed, Jun 17, 2020 at 11:41 AM Greg KH <greg@kroah.com> wrote:

> On Wed, Jun 17, 2020 at 11:17:28AM +0200, Tomek The Messenger wrote:
> > OK, so I had to wrongly described something because in my company this is
> > popular approach to use /sys as some abstract layer to get access to
> > registers.
>
> Please do not do that.  There are valid kernel apis to grant access to
> registers easily, and creating random files and directories in the root
> of sysfs is not only the incorrect way of doing it, you are making more
> work for yourself to do so, adding more code that is not needed.
>
> Do you have a pointer to the code so that we can provide help with
> showing the "correct" way to do all of this?
>
> thanks,
>
> greg k-h
>

[-- Attachment #1.2: Type: text/html, Size: 1198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Valdis Klētnieks]

[-- Attachment #1.1: Type: text/plain, Size: 253 bytes --]

On Wed, 17 Jun 2020 12:39:08 +0200, Tomek The Messenger said:

> OK, I will ask if it is possible to share the code.

Well... if you planned to ship that code and that hardware outside
your organization, it's going to have to be GPLv2-compatible.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Greg KH]

On Wed, Jun 17, 2020 at 08:24:14AM -0400, Valdis Klētnieks wrote:
> On Wed, 17 Jun 2020 12:39:08 +0200, Tomek The Messenger said:
> 
> > OK, I will ask if it is possible to share the code.
> 
> Well... if you planned to ship that code and that hardware outside
> your organization, it's going to have to be GPLv2-compatible.

It already is, if it is using kobjects directly :)

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Martin Kaiser]

Hello Greg and all,

Thus wrote Greg KH (greg@kroah.com):

> Please do not do that. There are valid kernel apis to grant access to
> registers easily,

the most simple case would be a "reset reason" register within the
chip's address space. A hand-crafted driver would ioremap the region and
implement a sysfs show method that reads the register.

What do you recommend for providing read access to such a register
via sysfs? Is this a job for the userspace I/O (UIO) subsystem?

Thanks,
Martin

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 1484 bytes --]

This is the case about which Martin write shortly. Then let's assume on
another soc reset reason is not stored in chip's address space memory
mapped to address 0xfff.... but it is accessed via some spi operation. On
another soc reset reason is still memory mapped but to different address
0xfff... And then let's assume there are 30-40 such functionalities like
reset_reason. If we have one unique sysfs interface then it is easier for
everyone: tester, developer to proceed with such device, testing or
debugging. You don't waste time to read documentation each time, checking
what address is/how to read each functionality. You have this hidden in
kernel modules code and can access via cat/echo to /sys.

On Wed, Jun 17, 2020 at 2:36 PM Martin Kaiser <lists@kaiser.cx> wrote:

> Hello Greg and all,
>
> Thus wrote Greg KH (greg@kroah.com):
>
> > Please do not do that. There are valid kernel apis to grant access to
> > registers easily,
>
> the most simple case would be a "reset reason" register within the
> chip's address space. A hand-crafted driver would ioremap the region and
> implement a sysfs show method that reads the register.
>
> What do you recommend for providing read access to such a register
> via sysfs? Is this a job for the userspace I/O (UIO) subsystem?
>
> Thanks,
> Martin
>
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies@kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>

[-- Attachment #1.2: Type: text/html, Size: 2093 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Valdis Klētnieks]

[-- Attachment #1.1: Type: text/plain, Size: 1345 bytes --]

On Wed, 17 Jun 2020 14:48:36 +0200, Tomek The Messenger said:

> This is the case about which Martin write shortly. Then let's assume on
> another soc reset reason is not stored in chip's address space memory
> mapped to address 0xfff.... but it is accessed via some spi operation. On
> another soc reset reason is still memory mapped but to different address
> 0xfff... And then let's assume there are 30-40 such functionalities like
> reset_reason.

Or you could hire hardware engineers who don't create designs that
look like they were done by crack-addicted howler monkeys, but instead
did sane designs that were easier to program. You know, like being
consistent with whether the registers are SPI or not, and so on.

> If we have one unique sysfs interface then it is easier for
> everyone: tester, developer to proceed with such device, testing or
> debugging.

No - not really.  Because if you're mapping 3 or 4 SOC resets reasons to
one thing, you then have to write code that says "If it's SoC 1, then
a bit 12 being set was a memory error, but if it's SOC 2 bit 12 means a
PCI bus reset and a memory parity error is bit 17, but a memory ECC
error is bit 4."

And if the meaning of the reset register is different, a lot of *other* things
are probably different too, which is an argument for just having separate
drivers for each SoC.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Greg KH]

On Wed, Jun 17, 2020 at 02:35:28PM +0200, Martin Kaiser wrote:
> Hello Greg and all,
> 
> Thus wrote Greg KH (greg@kroah.com):
> 
> > Please do not do that. There are valid kernel apis to grant access to
> > registers easily,
> 
> the most simple case would be a "reset reason" register within the
> chip's address space. A hand-crafted driver would ioremap the region and
> implement a sysfs show method that reads the register.

No, that's not what sysfs is for.  You would export this in the proper
way that "reset reason" expects.

> What do you recommend for providing read access to such a register
> via sysfs? Is this a job for the userspace I/O (UIO) subsystem?

If all you want is direct memory access to userspace for stuff like
this, what's wrong with /dev/mem ?

If you want access to memory and interrupts to do it all in userspace,
then yes, that's what UIO is for, use that please.

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Ahmad Fatoum]

Hello,

On 6/17/20 2:48 PM, Tomek The Messenger wrote:
> This is the case about which Martin write shortly. Then let's assume on
> another soc reset reason is not stored in chip's address space memory
> mapped to address 0xfff.... but it is accessed via some spi operation. On
> another soc reset reason is still memory mapped but to different address
> 0xfff... And then let's assume there are 30-40 such functionalities like
> reset_reason. If we have one unique sysfs interface then it is easier for
> everyone: tester, developer to proceed with such device, testing or
> debugging. You don't waste time to read documentation each time, checking
> what address is/how to read each functionality. You have this hidden in
> kernel modules code and can access via cat/echo to /sys.

It also gets more interesting when you have multiple reset reasons in the
same system, e.g. SoC power control differentiates between POR, external and
Watchdog reset and Watchdog OTOH differentiates between watchdog reset and
regular reset and then you might have an external PMIC with an integrated
watchdog that overrides both.

In projects I've been involved in, the bootloader did the boot reason computation[1]
and was the one to act on it, so communication to the kernel wasn't of high
importance. It would be great to have:

1) an upstream device tree binding for reset reason communication from bootloader
   to Linux
2) a mainline User API to query the reset reason
3) a mainline in-kernel API for drivers to set reset reasons

Watchdog drivers already have a WDIOF_CARDRESET flag that could be folded into
this. If you feel up to the task and go along with upstreaming it, you'll
save yourself (and future colleagues who need to debug locking problems between
your module and mainline kernel drivers) a lot of hassle.

[1]: https://barebox.org/doc/latest/user/reset-reason.html

Cheers
Ahmad


> 
> On Wed, Jun 17, 2020 at 2:36 PM Martin Kaiser <lists@kaiser.cx> wrote:
> 
>> Hello Greg and all,
>>
>> Thus wrote Greg KH (greg@kroah.com):
>>
>>> Please do not do that. There are valid kernel apis to grant access to
>>> registers easily,
>>
>> the most simple case would be a "reset reason" register within the
>> chip's address space. A hand-crafted driver would ioremap the region and
>> implement a sysfs show method that reads the register.
>>
>> What do you recommend for providing read access to such a register
>> via sysfs? Is this a job for the userspace I/O (UIO) subsystem?
>>
>> Thanks,
>> Martin
>>
>> _______________________________________________
>> Kernelnewbies mailing list
>> Kernelnewbies@kernelnewbies.org
>> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>>
> 
> 
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies@kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Tomek The Messenger]

[-- Attachment #1.1: Type: text/plain, Size: 4679 bytes --]

> If we have one unique sysfs interface then it is easier for
> everyone: tester, developer to proceed with such device, testing or
> debugging.

>>No - not really.  Because if you're mapping 3 or 4 SOC resets reasons to
>>one thing, you then have to write code that says "If it's SoC 1, then
>>a bit 12 being set was a memory error, but if it's SOC 2 bit 12 means a
>>PCI bus reset and a memory parity error is bit 17, but a memory ECC
>>error is bit 4."

>>And if the meaning of the reset register is different, a lot of *other*
things
>>are probably different too, which is an argument for just having separate
>>drivers for each SoC.

Yes separate drivers but the actions do by them is to create the same files
in sysfs. So if somebody use intel, texas instrument or xilinx soc then in
order to read reset reason or other stuff he will use sysfs. So this will
look  likes:
/sys/resetreg - 3 separate drivers plus  one core driver where there  is
common  part
/sys/otherfunctionality- as Above
Apart from  above 3 drivers where there is api to access  registers from
ti,  Intel, xilinx. This was original  question if  place here all api
reads/writes or only common between sysfs kernel modules.
...
/sys/functionalityN  -> separate driver for intel, TI, Xilinx

On Wed, Jun 17, 2020 at 3:34 PM Ahmad Fatoum <a.fatoum@pengutronix.de>
wrote:

> Hello,
>
> On 6/17/20 2:48 PM, Tomek The Messenger wrote:
> > This is the case about which Martin write shortly. Then let's assume on
> > another soc reset reason is not stored in chip's address space memory
> > mapped to address 0xfff.... but it is accessed via some spi operation. On
> > another soc reset reason is still memory mapped but to different address
> > 0xfff... And then let's assume there are 30-40 such functionalities like
> > reset_reason. If we have one unique sysfs interface then it is easier for
> > everyone: tester, developer to proceed with such device, testing or
> > debugging. You don't waste time to read documentation each time, checking
> > what address is/how to read each functionality. You have this hidden in
> > kernel modules code and can access via cat/echo to /sys.
>
> It also gets more interesting when you have multiple reset reasons in the
> same system, e.g. SoC power control differentiates between POR, external
> and
> Watchdog reset and Watchdog OTOH differentiates between watchdog reset and
> regular reset and then you might have an external PMIC with an integrated
> watchdog that overrides both.
>
> In projects I've been involved in, the bootloader did the boot reason
> computation[1]
> and was the one to act on it, so communication to the kernel wasn't of high
> importance. It would be great to have:
>
> 1) an upstream device tree binding for reset reason communication from
> bootloader
>    to Linux
> 2) a mainline User API to query the reset reason
> 3) a mainline in-kernel API for drivers to set reset reasons
>
> Watchdog drivers already have a WDIOF_CARDRESET flag that could be folded
> into
> this. If you feel up to the task and go along with upstreaming it, you'll
> save yourself (and future colleagues who need to debug locking problems
> between
> your module and mainline kernel drivers) a lot of hassle.
>
> [1]: https://barebox.org/doc/latest/user/reset-reason.html
>
> Cheers
> Ahmad
>
>
> >
> > On Wed, Jun 17, 2020 at 2:36 PM Martin Kaiser <lists@kaiser.cx> wrote:
> >
> >> Hello Greg and all,
> >>
> >> Thus wrote Greg KH (greg@kroah.com):
> >>
> >>> Please do not do that. There are valid kernel apis to grant access to
> >>> registers easily,
> >>
> >> the most simple case would be a "reset reason" register within the
> >> chip's address space. A hand-crafted driver would ioremap the region and
> >> implement a sysfs show method that reads the register.
> >>
> >> What do you recommend for providing read access to such a register
> >> via sysfs? Is this a job for the userspace I/O (UIO) subsystem?
> >>
> >> Thanks,
> >> Martin
> >>
> >> _______________________________________________
> >> Kernelnewbies mailing list
> >> Kernelnewbies@kernelnewbies.org
> >> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> >>
> >
> >
> > _______________________________________________
> > Kernelnewbies mailing list
> > Kernelnewbies@kernelnewbies.org
> > https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>

[-- Attachment #1.2: Type: text/html, Size: 6616 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Greg KH]

On Wed, Jun 17, 2020 at 04:26:49PM +0200, Tomek The Messenger wrote:
> > If we have one unique sysfs interface then it is easier for
> > everyone: tester, developer to proceed with such device, testing or
> > debugging.
> 
> >>No - not really.  Because if you're mapping 3 or 4 SOC resets reasons to
> >>one thing, you then have to write code that says "If it's SoC 1, then
> >>a bit 12 being set was a memory error, but if it's SOC 2 bit 12 means a
> >>PCI bus reset and a memory parity error is bit 17, but a memory ECC
> >>error is bit 4."
> 
> >>And if the meaning of the reset register is different, a lot of *other*
> things
> >>are probably different too, which is an argument for just having separate
> >>drivers for each SoC.
> 
> Yes separate drivers but the actions do by them is to create the same files
> in sysfs. So if somebody use intel, texas instrument or xilinx soc then in
> order to read reset reason or other stuff he will use sysfs. So this will
> look  likes:
> /sys/resetreg - 3 separate drivers plus  one core driver where there  is
> common  part
> /sys/otherfunctionality- as Above
> Apart from  above 3 drivers where there is api to access  registers from
> ti,  Intel, xilinx. This was original  question if  place here all api
> reads/writes or only common between sysfs kernel modules.
> ...
> /sys/functionalityN  -> separate driver for intel, TI, Xilinx


Again, use the specific subsystem for these values, don't try to expose
your own mess in sysfs, especially with "raw" kobjects.  You will run
into loads of problems that way.

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: the cost of EXPORT_SYMBOL_GPL

[Valdis Klētnieks]

[-- Attachment #1.1: Type: text/plain, Size: 563 bytes --]

On Wed, 17 Jun 2020 16:26:49 +0200, Tomek The Messenger said:

> Yes separate drivers but the actions do by them is to create the same files
> in sysfs. So if somebody use intel, texas instrument or xilinx soc then in
> order to read reset reason or other stuff he will use sysfs. So this will
> look  likes:
> /sys/resetreg - 3 separate drivers plus  one core driver where there  is
> common  part

That should be /sys/something/something/resetreg where the somethings
put it in the same place in the /sys tree as all the other sysfs entries for
your devices.



[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies