| Message ID | 20200324150847.10476-3-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc-bios: s390x: Cleanup part 1 | expand |
On 24.03.20 16:08, Janosch Frank wrote: > If we have a lowcore struct that has members for offsets that we want > to touch, why not use it? > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > pc-bios/s390-ccw/cio.h | 17 +++++++++++------ > pc-bios/s390-ccw/main.c | 8 +++----- > 2 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h > index aaa432deddb9242e..9ad794a789c47df2 100644 > --- a/pc-bios/s390-ccw/cio.h > +++ b/pc-bios/s390-ccw/cio.h > @@ -122,12 +122,17 @@ typedef struct schib { > } __attribute__ ((packed, aligned(4))) Schib; > > typedef struct subchannel_id { > - __u32 cssid:8; > - __u32:4; > - __u32 m:1; > - __u32 ssid:2; > - __u32 one:1; > - __u32 sch_no:16; > + union { > + struct { > + __u16 cssid:8; > + __u16:4; Broken indentation. Also, do we want to give that strange-looking "__u32:4" a name ("reserved"). > + __u16 m:1; > + __u16 ssid:2; > + __u16 one:1; > + }; > + __u16 sch_id; > + }; > + __u16 sch_no; > } __attribute__ ((packed, aligned(4))) SubChannelId; > > struct chsc_header { > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > index 4e65b411e1d890ba..8b912454c940a390 100644 > --- a/pc-bios/s390-ccw/main.c > +++ b/pc-bios/s390-ccw/main.c > @@ -36,11 +36,9 @@ LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ > */ > void write_subsystem_identification(void) > { > - SubChannelId *schid = (SubChannelId *) 184; > - uint32_t *zeroes = (uint32_t *) 188; > - > - *schid = blk_schid; > - *zeroes = 0; > + lowcore->subchannel_id = blk_schid.sch_id; > + lowcore->subchannel_nr = blk_schid.sch_no; > + lowcore->io_int_parm = 0; > } > > void write_iplb_location(void) > Looks sane to me.
On 3/27/20 11:25 AM, David Hildenbrand wrote: > On 24.03.20 16:08, Janosch Frank wrote: >> If we have a lowcore struct that has members for offsets that we want >> to touch, why not use it? >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> pc-bios/s390-ccw/cio.h | 17 +++++++++++------ >> pc-bios/s390-ccw/main.c | 8 +++----- >> 2 files changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h >> index aaa432deddb9242e..9ad794a789c47df2 100644 >> --- a/pc-bios/s390-ccw/cio.h >> +++ b/pc-bios/s390-ccw/cio.h >> @@ -122,12 +122,17 @@ typedef struct schib { >> } __attribute__ ((packed, aligned(4))) Schib; >> >> typedef struct subchannel_id { >> - __u32 cssid:8; >> - __u32:4; >> - __u32 m:1; >> - __u32 ssid:2; >> - __u32 one:1; >> - __u32 sch_no:16; >> + union { >> + struct { >> + __u16 cssid:8; >> + __u16:4; > > Broken indentation. Also, do we want to give that strange-looking > "__u32:4" a name ("reserved"). Fixed indentation and renamed. > >> + __u16 m:1; >> + __u16 ssid:2; >> + __u16 one:1; >> + }; >> + __u16 sch_id; >> + }; >> + __u16 sch_no; >> } __attribute__ ((packed, aligned(4))) SubChannelId; >> >> struct chsc_header { >> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c >> index 4e65b411e1d890ba..8b912454c940a390 100644 >> --- a/pc-bios/s390-ccw/main.c >> +++ b/pc-bios/s390-ccw/main.c >> @@ -36,11 +36,9 @@ LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ >> */ >> void write_subsystem_identification(void) >> { >> - SubChannelId *schid = (SubChannelId *) 184; >> - uint32_t *zeroes = (uint32_t *) 188; >> - >> - *schid = blk_schid; >> - *zeroes = 0; >> + lowcore->subchannel_id = blk_schid.sch_id; >> + lowcore->subchannel_nr = blk_schid.sch_no; >> + lowcore->io_int_parm = 0; >> } >> >> void write_iplb_location(void) >> > > Looks sane to me. >
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h index aaa432deddb9242e..9ad794a789c47df2 100644 --- a/pc-bios/s390-ccw/cio.h +++ b/pc-bios/s390-ccw/cio.h @@ -122,12 +122,17 @@ typedef struct schib { } __attribute__ ((packed, aligned(4))) Schib; typedef struct subchannel_id { - __u32 cssid:8; - __u32:4; - __u32 m:1; - __u32 ssid:2; - __u32 one:1; - __u32 sch_no:16; + union { + struct { + __u16 cssid:8; + __u16:4; + __u16 m:1; + __u16 ssid:2; + __u16 one:1; + }; + __u16 sch_id; + }; + __u16 sch_no; } __attribute__ ((packed, aligned(4))) SubChannelId; struct chsc_header { diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index 4e65b411e1d890ba..8b912454c940a390 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -36,11 +36,9 @@ LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ */ void write_subsystem_identification(void) { - SubChannelId *schid = (SubChannelId *) 184; - uint32_t *zeroes = (uint32_t *) 188; - - *schid = blk_schid; - *zeroes = 0; + lowcore->subchannel_id = blk_schid.sch_id; + lowcore->subchannel_nr = blk_schid.sch_no; + lowcore->io_int_parm = 0; } void write_iplb_location(void)
If we have a lowcore struct that has members for offsets that we want to touch, why not use it? Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- pc-bios/s390-ccw/cio.h | 17 +++++++++++------ pc-bios/s390-ccw/main.c | 8 +++----- 2 files changed, 14 insertions(+), 11 deletions(-)