On 3/8/21 3:01 PM, Pierre Morel wrote: > > > On 3/1/21 3:45 PM, Janosch Frank wrote: >> On 3/1/21 12:47 PM, Pierre Morel wrote: >>> CSS characteristics exposes the features of the Channel SubSystem. >>> Let's use Store Channel Subsystem Characteristics to retrieve >>> the features of the CSS. >>> >>> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx> >>> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx> >> >> Acked-by: Janosch Frank <frankja@xxxxxxxxxxxxx> >> >> Small nits below >> >>> --- >>> lib/s390x/css.h | 67 ++++++++++++++++++++++++++++++++ >>> lib/s390x/css_lib.c | 93 ++++++++++++++++++++++++++++++++++++++++++++- >>> s390x/css.c | 8 ++++ >>> 3 files changed, 167 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h >>> index 3e57445..4210472 100644 >>> --- a/lib/s390x/css.h >>> +++ b/lib/s390x/css.h >>> @@ -288,4 +288,71 @@ int css_residual_count(unsigned int schid); >>> void enable_io_isc(uint8_t isc); >>> int wait_and_check_io_completion(int schid); >>> >>> +/* >>> + * CHSC definitions >>> + */ >>> +struct chsc_header { >>> + uint16_t len; >>> + uint16_t code; >>> +}; >>> + >>> +/* Store Channel Subsystem Characteristics */ >>> +struct chsc_scsc { >>> + struct chsc_header req; >>> + uint16_t req_fmt; >>> + uint8_t cssid; >>> + uint8_t res_03; >>> + uint32_t res_04[2]; >> >> I find the naming a bit weird and it could be one uint8_t field. > > OK > >> >>> + struct chsc_header res; >>> + uint32_t res_fmt; >>> + uint64_t general_char[255]; >>> + uint64_t chsc_char[254]; >>> +}; > > ... snip > >>> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c >>> index 3c24480..f46e871 100644 >>> --- a/lib/s390x/css_lib.c >>> +++ b/lib/s390x/css_lib.c >>> @@ -15,11 +15,102 @@ >>> #include <asm/arch_def.h> >>> #include <asm/time.h> >>> #include <asm/arch_def.h> >>> - >>> +#include <alloc_page.h> >> >> Did you intend to remove the newline here? > > Yes I do not see why we should have a new line here. Some people like to separate asm includes from other includes. > But I can keep it if you want. I just wanted to know if you removed it by mistake, if you want it that way it's ok for me. > >> > ... snip... > >>> #include <asm/arch_def.h> >>> +#include <alloc_page.h> >>> >>> #include <malloc_io.h> >>> #include <css.h> >>> @@ -140,10 +141,17 @@ error_senseid: >>> unregister_io_int_func(css_irq_io); >>> } >>> >>> +static void css_init(void) >>> +{ >>> + report(!!get_chsc_scsc(), "Store Channel Characteristics"); >> >> get_chsc_scsc() returns a bool, so you shouldn't need the !! here, no? > > Yes, forgotten when changed get_chsc_scsc(), remove the !! > > > Thanks, > Pierre >