From: Greg KH <gregkh@linuxfoundation.org>
To: "Czerwacki, Eial" <eial.czerwacki@sap.com>
Cc: "linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
"Hammer, Gal" <gal.hammer@sap.com>,
SAP vSMP Linux Maintainer <linux.vsmp@sap.com>
Subject: Re: invalid drv data in show attribute
Date: Mon, 5 Sep 2022 14:10:55 +0200 [thread overview]
Message-ID: <YxXnT42bNR0got/E@kroah.com> (raw)
In-Reply-To: <PAXPR02MB73100F851CC17B941A5CA49D817F9@PAXPR02MB7310.eurprd02.prod.outlook.com>
On Mon, Sep 05, 2022 at 12:02:47PM +0000, Czerwacki, Eial wrote:
> >On Mon, Sep 05, 2022 at 07:28:02AM +0000, Czerwacki, Eial wrote:
> >> >On Mon, Sep 05, 2022 at 06:07:07AM +0000, Czerwacki, Eial wrote:
> >> >> >
> >> >> >On Sun, Sep 04, 2022 at 02:37:32PM +0000, Czerwacki, Eial wrote:
> >> >> >> Greetings,
> >> >> >>
> >> >> >> while working on a driver, I've found a bug that I'm unable to understand.
> >> >> >> I assume that I'm doing something wrong. here is my reduced c file:
> >> >> >
> >> >> ><snip>
> >> >> >
> >> >> >I'll provide a better review after my coffee, but just one comment
> >> >> >first. Ok, two:
> >> >> >
> >> >> >> #ifndef sysfs_emit
> >> >> >> #define sysfs_emit sprintf
> >> >> >> #endif // sysfs_emit
> >> >> >
> >> >> >Wait what? You mention at the end that you do nto have sysfs_emit in
> >> >> >your kernel tree, but all activly maintained kernels does have this
> >> >> >function. You should NEVER be working on a kernel tree that is not
> >> >> >actually supported, and for new code like you are wanting to submit, you
> >> >> >should always work on Linus's tree, or the last release, or something
> >> >> >newer.
> >> >> >
> >> >> >Please move to 5.19 now, it will save you so much time later on...
> >> >> well, I'm kinda binded to this kernel version buy you are right.
> >> >
> >> >What kernel version does not have sysfs_emit()?
> >> SELS 15 SP2, it uses kernel 5.3.18-24.67
> >
> >Ick, never work on an enterprise kernel for new stuff, they are crazy
> >and you will need to usually redo your whole thing for upstream in the
> >end.
> >
> >Just work off of the latest tree please and then backport when needed.
> understood, that is the main focus now
>
> >
> >> >> >Write the Documentation/ABI/ entries first, what do they look like for
> >> >> >your new sysfs files?
> >> >>
> >> >> I thought that is my issue but wasn't sure.
> >> >> what I'm looking for is this tree:
> >> >> - vsmp
> >> >> -- version
> >> >> -- summery
> >> >> --- data#1
> >> >> ...
> >> >> --- data#n
> >> >> -- boards
> >> >> --- 0
> >> >> ---- data#1
> >> >> ...
> >> >> ---- data#k
> >> >> --- 1
> >> >> ...
> >> >> --- l
> >> >>
> >> >> each board has a predefine set of attributes when I need to add another depending on the type.
> >> >> also there are shared attributes between summery folder and the boards. that I was able to implement based on the name of the entry
> >> >
> >> >So "boards" are devices, and then you need a bus to manage them. Are
> >> >you sure you need/want all of this?
> >> no, boards are not devices, they logical partitions inside the hypervisor.
> >
> >Which can be a device, we have loads of virtual devices, it's how the
> >driver model works.
> >
> >> each board has its own data I'd like to export.
> >
> >Then they should be treated as a device.
> not sure I understand how they are connected, is there an example I can look at?
The kernel has loads of busses and classes as real-world examples :)
Think of it this way, each of your board will have a `struct device` in
it, and then you will add them to the system. As you don't really have
a complex thing, each board will just be bound to the "same" driver, and
your bus will only have one driver, but that will allow them all to be
enumerated and handled easily.
And they will all hang off of the PCI device that you had here, it's up
to you if you want to make a "bus device" in the middle or not like many
busses do, but that's getting ahead a bit I think.
Go read the documentation and try it out, there's a lot of concepts here
that I think you need to learn up on in order to make this work well.
Sorry it's not the simplest thing, but then again, it is an operating
system :)
And aren't there others at your company that can help you with this?
> >Great, make them devices.
> I still need to understand how to make them as devices.
> if I understood you correctly, your suggestion is to run modprobe vsmp and there will be virtual device per board?
> then for each I'll create the relevant sysfs entries?
> if so, where can I find the global sysfs entries?
What do you mean by "global" here? Just put those below /sys/hypervisor
if you want, that should be simple, but not for the individual devices.
> also, how can organize the entries so they will appear under one location which is static?
The kernel device tree is never static.
> I'd prefer not adding complex code to the utils which need to traverse the sysfs tree to collect the right data
for device in $(ls /sys/bus/vsmp/device/); do
that's not that hard :)
thanks,
greg k-h
next prev parent reply other threads:[~2022-09-05 12:10 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-04 14:37 invalid drv data in show attribute Czerwacki, Eial
2022-09-05 5:49 ` Greg KH
2022-09-05 6:07 ` Czerwacki, Eial
2022-09-05 7:01 ` Greg KH
2022-09-05 7:28 ` Czerwacki, Eial
2022-09-05 11:41 ` Greg KH
2022-09-05 12:02 ` Czerwacki, Eial
2022-09-05 12:10 ` Greg KH [this message]
2022-09-05 12:56 ` Czerwacki, Eial
2022-09-05 15:12 ` Greg KH
2022-09-05 15:52 ` Czerwacki, Eial
2022-09-05 16:02 ` Greg KH
2022-09-06 7:30 ` Czerwacki, Eial
2022-09-06 12:09 ` Greg KH
2022-09-06 12:28 ` Czerwacki, Eial
2022-09-05 7:07 ` Greg KH
2022-09-05 7:44 ` Czerwacki, Eial
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YxXnT42bNR0got/E@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=eial.czerwacki@sap.com \
--cc=gal.hammer@sap.com \
--cc=linux-staging@lists.linux.dev \
--cc=linux.vsmp@sap.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox