| Message ID | 20210924150424.10047-2-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | include/public: update usbif.h | expand |
On 24.09.2021 17:04, Juergen Gross wrote: > The interface definition of PV USB devices is lacking the specification > of possible values of the status filed in a response. Those are Nit: "field"? > negative errno values as used in Linux, so they might differ in other > OS's. Specify them via appropriate defines. What if new errno values got used by the driver? Would we alter the public header every time? Or is the likelihood of further values ever getting used vanishingly small? In how far would it be possible to tie these to Xen's public/errno.h instead? > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > xen/include/public/io/usbif.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h > index c6a58639d6..fbd6f953f8 100644 > --- a/xen/include/public/io/usbif.h > +++ b/xen/include/public/io/usbif.h > @@ -221,6 +221,13 @@ struct usbif_urb_response { > uint16_t id; /* request id */ > uint16_t start_frame; /* start frame (ISO) */ > int32_t status; /* status (non-ISO) */ > +#define USBIF_STATUS_OK 0 > +#define USBIF_STATUS_NODEV -19 > +#define USBIF_STATUS_INVAL -22 > +#define USBIF_STATUS_STALL -32 > +#define USBIF_STATUS_IOERROR -71 > +#define USBIF_STATUS_BABBLE -75 > +#define USBIF_STATUS_SHUTDOWN -108 Nit: While probably benign for all practical uses, these negative values nevertheless would better be parenthesized. Jan
On 27.09.21 10:13, Jan Beulich wrote: > On 24.09.2021 17:04, Juergen Gross wrote: >> The interface definition of PV USB devices is lacking the specification >> of possible values of the status filed in a response. Those are > > Nit: "field"? Yes, of course. > >> negative errno values as used in Linux, so they might differ in other >> OS's. Specify them via appropriate defines. > > What if new errno values got used by the driver? Would we alter the > public header every time? Or is the likelihood of further values ever > getting used vanishingly small? In how far would it be possible to tie > these to Xen's public/errno.h instead? Those are the current values returned by the backend. Other ones used internally in the backend should IMO tried to be mapped to the values defined in the interface specification. > >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> xen/include/public/io/usbif.h | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h >> index c6a58639d6..fbd6f953f8 100644 >> --- a/xen/include/public/io/usbif.h >> +++ b/xen/include/public/io/usbif.h >> @@ -221,6 +221,13 @@ struct usbif_urb_response { >> uint16_t id; /* request id */ >> uint16_t start_frame; /* start frame (ISO) */ >> int32_t status; /* status (non-ISO) */ >> +#define USBIF_STATUS_OK 0 >> +#define USBIF_STATUS_NODEV -19 >> +#define USBIF_STATUS_INVAL -22 >> +#define USBIF_STATUS_STALL -32 >> +#define USBIF_STATUS_IOERROR -71 >> +#define USBIF_STATUS_BABBLE -75 >> +#define USBIF_STATUS_SHUTDOWN -108 > > Nit: While probably benign for all practical uses, these negative > values nevertheless would better be parenthesized. Okay, fine with me. Juergen
On 27.09.2021 10:20, Juergen Gross wrote: > On 27.09.21 10:13, Jan Beulich wrote: >> On 24.09.2021 17:04, Juergen Gross wrote: >>> The interface definition of PV USB devices is lacking the specification >>> of possible values of the status filed in a response. Those are >>> negative errno values as used in Linux, so they might differ in other >>> OS's. Specify them via appropriate defines. >> >> What if new errno values got used by the driver? Would we alter the >> public header every time? Or is the likelihood of further values ever >> getting used vanishingly small? In how far would it be possible to tie >> these to Xen's public/errno.h instead? > > Those are the current values returned by the backend. Other ones used > internally in the backend should IMO tried to be mapped to the values > defined in the interface specification. In which case I'd like to reword my question: Is the set of values added sufficiently rich? There's e.g. no ENOMEM equivalent. One other observation: Why is this header using tab indentation? This is by far the largest (albeit sadly not the only) of the style violators in xen/include/public/io/. Jan
On 27.09.21 10:27, Jan Beulich wrote: > On 27.09.2021 10:20, Juergen Gross wrote: >> On 27.09.21 10:13, Jan Beulich wrote: >>> On 24.09.2021 17:04, Juergen Gross wrote: >>>> The interface definition of PV USB devices is lacking the specification >>>> of possible values of the status filed in a response. Those are >>>> negative errno values as used in Linux, so they might differ in other >>>> OS's. Specify them via appropriate defines. >>> >>> What if new errno values got used by the driver? Would we alter the >>> public header every time? Or is the likelihood of further values ever >>> getting used vanishingly small? In how far would it be possible to tie >>> these to Xen's public/errno.h instead? >> >> Those are the current values returned by the backend. Other ones used >> internally in the backend should IMO tried to be mapped to the values >> defined in the interface specification. > > In which case I'd like to reword my question: Is the set of values added > sufficiently rich? There's e.g. no ENOMEM equivalent. Those are mapped to ESHUTDOWN today. > One other observation: Why is this header using tab indentation? This is > by far the largest (albeit sadly not the only) of the style violators in > xen/include/public/io/. Good question. This seems to date back to its introduction back in 2009. I can add a style fixup patch to the series. Juergen
diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h index c6a58639d6..fbd6f953f8 100644 --- a/xen/include/public/io/usbif.h +++ b/xen/include/public/io/usbif.h @@ -221,6 +221,13 @@ struct usbif_urb_response { uint16_t id; /* request id */ uint16_t start_frame; /* start frame (ISO) */ int32_t status; /* status (non-ISO) */ +#define USBIF_STATUS_OK 0 +#define USBIF_STATUS_NODEV -19 +#define USBIF_STATUS_INVAL -22 +#define USBIF_STATUS_STALL -32 +#define USBIF_STATUS_IOERROR -71 +#define USBIF_STATUS_BABBLE -75 +#define USBIF_STATUS_SHUTDOWN -108 int32_t actual_length; /* actual transfer length */ int32_t error_count; /* number of ISO errors */ };
The interface definition of PV USB devices is lacking the specification of possible values of the status filed in a response. Those are negative errno values as used in Linux, so they might differ in other OS's. Specify them via appropriate defines. Signed-off-by: Juergen Gross <jgross@suse.com> --- xen/include/public/io/usbif.h | 7 +++++++ 1 file changed, 7 insertions(+)