Re: [Openipmi-developer] [PATCH v7 1/3] ipmi: ssif_bmc: Add SSIF BMC driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/06/2022 07:32, Corey Minyard wrote:
On Wed, Jun 01, 2022 at 03:23:11PM +0700, Quan Nguyen wrote:
On 04/05/2022 19:06, Corey Minyard wrote:
On Wed, May 04, 2022 at 01:45:03PM +0700, Quan Nguyen via Openipmi-developer wrote:

I seem to remember mentioning this before, but there is no reason to
pack the structures below.


The packed structure is because we want to pick the len directly from user
space without worry about the padding byte.

As we plan not to use the .h file in next version, I still would like to use
packed structure internally inside ssif_bmc.c file.

Packed doesn't matter for the userspace API.  If you look at other
structures in the userspace API, they are not packed, either.  The
compiler will do the right thing on both ends.


And second, the following is a userspace API structures, so it needs to
be in its own file in include/uapi/linux, along with any supporting
things that users will need to use.  And your userspace code should be
using that file.


Meantime, I'd like not to use .h as I see there is no demand for sharing the
data structure between kernel and user space yet. But we may do it in the
future.

If you have a userspace API, it needs to be in include/uapi/linux.
You may not be the only user of this code.  In fact, you probably won't
be.  You need to have a .h with the structures in it, you don't want the
same structure in two places if you can help it.


Dear Corey,

Is it OK to push the structure definition into the
include/uapi/linux/ipmi_bmc.h ?

Or should it need to be in separate new header file in uapi/linux ?

I think a different file, like ipmi_ssif_bmc, to match the file and
operation.  Unless you need the things in ipmi_bmc.h, which I don't
think is the case.


Thanks Corey,
Will add ipmi_ssif_bmc as you suggested.

-- Quan



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux