On Thu, Mar 17, 2022 at 08:17:04AM +0000, Czerwacki, Eial wrote: > Greetings Greg, > > >On Thu, Mar 17, 2022 at 07:34:30AM +0000, Czerwacki, Eial wrote: > >> Greetings Greg, > >> > >> >On Wed, Mar 16, 2022 at 06:13:04PM +0000, Czerwacki, Eial wrote: > >> >> Introducing the vSMP guest driver which allows interaction with the vSMP control device when > >> >> running a Linux OS atop of the vSMP hypervisor. > >> >> vSMP is a resource aggregation hypervisor from SAP. > >> >> > >> >> the driver comprises of 3 modules, vsmp which includes all the api needed to interact with the > >> >> control driver, vsmp_logs which allows reading logs from the hypervisor and vsmp_common_info which > >> >> allows reading generic information the hypervisor exposes, currently only the version is exposed. > >> > > >> >Please wrap changelog text at 72 columns, like git asks you to. > >> git didn't asked me to do so, I'll fix it in the next iteration. > >> > >> > > >> >Also, why did you not cc: the staging maintainer? :( > >> I've probably missed that when I was going over the docs, my bad. > >> > >> > > >> >> Signed-off-by: Eial Czerwacki <eial.czerwacki@xxxxxxx> > >> >> --- > >> >> MAINTAINERS | 6 + > >> >> drivers/staging/Kconfig | 2 + > >> >> drivers/staging/Makefile | 1 + > >> >> drivers/staging/vsmp/Kconfig | 14 + > >> >> drivers/staging/vsmp/Makefile | 7 + > >> >> drivers/staging/vsmp/api.c | 402 ++++++++++++++++++++++++ > >> >> drivers/staging/vsmp/api.h | 61 ++++ > >> >> drivers/staging/vsmp/common/Kconfig | 11 + > >> >> drivers/staging/vsmp/common/Makefile | 7 + > >> >> drivers/staging/vsmp/common/common.c | 64 ++++ > >> >> drivers/staging/vsmp/common/common.h | 27 ++ > >> >> drivers/staging/vsmp/common/version.c | 85 +++++ > >> >> drivers/staging/vsmp/logs/Kconfig | 10 + > >> >> drivers/staging/vsmp/logs/Makefile | 7 + > >> >> drivers/staging/vsmp/logs/active_logs.c | 112 +++++++ > >> >> drivers/staging/vsmp/registers.h | 16 + > >> > > >> >Without looking at the code, I do not see a TODO file here that lists > >> >the tasks that need to be completed to get this out of the > >> >drivers/staging/ directory. This is a requirement. > >> there are tasks to complete it however I'm not sure they are blockers. > > > >What tasks? > support of other information bits like stats I have no idea what that means :) > >> >Also, why is this submitted for drivers/staging/ ? What prevents it > >> >from being merged to the "correct" place in the kernel tree today? > >> afaiu, the correct order for new drivers is staging => mainline. > > > >No, not at all. This is the "hard way" to get drivers merged. I would > >never recommend doing it this way for anyone as it will take more time > >and effort than just doing it the correct way the first time. > I understand, thanks for clearing it up > > > > >> in addition, from past experience with the main kernel mailing list, I > >> thought the driver will get more traction here than in the main kernel > >> mailing list. unless there is a specific mailing list for drivers > >> beside the staging tree one > > > >There are zillions of subsystem mailing lists out there. Always target > >the specific one you are interested in. For this code, it should live > >in drivers/virt/ right? Ah, no mailing list specific for that one, but > >I usually am the one that merges the code in there, so cc: me and Arnd > >and lkml and we can go from there! > drivers/virt looks like the correct place, so I need to send it as [PATCH] to the lkml with Arnd and yourself in the CC of the mail? Yes please! > >Also, you have a bunch of sysfs files, all of those need to be > >documented in Documentation/ABI/ entries as you will see if you run > >scripts/get_abi.pl with your driver loaded. > will do. > > > > >Also, rip out the old-school PCI bus scanning stuff. That hasn't been > >needed since the 2.4 kernel days :) > can you point me to a doc that states how I can retrieve the dev struct of the device in question? The device in question is passed to you in your pci probe function callback. You do NOT allow any random PCI device number to be passed as a module parameter, as that way lies madness. If you MUST allow userspace to bind a specific PCI device to your driver, then use the sysfs "bind" file that the driver core provides to you. Then the normal pci probe callback will be called and all is good. In short, your driver is doing a lot of extra work that it doesn't need to do at all, just remove all of that code. thanks, greg k-h
