From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 51153E7D24B for ; Tue, 26 Sep 2023 08:10:28 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ql38t-0005Qs-9Q; Tue, 26 Sep 2023 04:10:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ql38o-0005Q8-LD for qemu-devel@nongnu.org; Tue, 26 Sep 2023 04:09:59 -0400 Received: from mout.kundenserver.de ([212.227.17.13]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ql38i-0000HM-TD for qemu-devel@nongnu.org; Tue, 26 Sep 2023 04:09:55 -0400 Received: from [192.168.100.1] ([82.142.8.70]) by mrelayeu.kundenserver.de (mreue109 [213.165.67.119]) with ESMTPSA (Nemesis) id 1MekvT-1rLUHi0K4z-00akdP; Tue, 26 Sep 2023 10:09:51 +0200 Message-ID: Date: Tue, 26 Sep 2023 10:09:50 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v2 12/20] swim: split into separate IWM and ISM register blocks Content-Language: fr To: Mark Cave-Ayland , qemu-devel@nongnu.org References: <20230909094827.33871-1-mark.cave-ayland@ilande.co.uk> <20230909094827.33871-13-mark.cave-ayland@ilande.co.uk> From: Laurent Vivier In-Reply-To: <20230909094827.33871-13-mark.cave-ayland@ilande.co.uk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K1:rPtwFT/2c0z65GwwiSxONRSn/E70SXaOriqcqwUx9vo5c6mPrYx g31Ire16+XdrtxBG5vsJoWwUgGbB9RRd+rhb6oF37XcY2AjeTQrLgtlKv3h/6iMAt0t80u+ 7xEvE7d19rVQsiHgFgtbT0OVEiNwZQCffXj7kQgNCCbvDGxd7ZaJEFQbLjuVQ9urAfphO3O gCoWOldNTgJSfj3BlTMXg== UI-OutboundReport: notjunk:1;M01:P0:KUHAGbxNIuQ=;B/zr0pP742i49JOJFIQn36d/7Rv 0tp4kCYh/RD/ld9pQ7pXZVUMjBXXPq4K1xRSQM9Z234sDNg8wYGCxMr3aqAjUrC+Dz/1TbhLv TyPwIZvV7JPP7QZEkqegNPHLZWehrwbAi/5RtdabzxLS4CKN4LkElrcJ+17wn6FKWyANDVtaq LDMbQ8Tz5N6mWB3CW/B3Y+J6gpFBwe625EN2rA4h2+1v1UWPySacDAr6djaZMj8wjDfWsSApi WDbjTIzQHonKXHMoPSyy1f2IDFMsEkaANXIjgNl9Az7+9J61M9wn4xWurKC3HGEWGF9fewdMN mtNGEY33RDCE4Qhz93UEbVZPuvTngCgiM7nobmmgock2co9CX+7qRd3vKnUu1LMXAmTI3CDNM pinGWOV4YJmRDnFMWj61w1ten62Odx9l6owUcHhksbyG1x+ZXD1rur6t6Oi8q2dSA0ts+rIom njiohT+D3XLeKLJnv7054+mfrnjza7LlBl+yyVtlY3TdLh8A15HSBPbQD0H5ADSC+1Avn7M6L OsZjsSmKid9UCOBID6koknzoneTvy+NFdGh44gzELuSlM4ZGsA/3FdPDBU3jP+7fNgoRnGt4J 8JBTNkm9piyx9sJk15arBRZu3GmZGeGZ75hK8zjGTJ0tWvC1zRGWvDrhUvUT1F5qr6QhMV3iT 9yeTKz2yzJTI0ohtG+ElY9+3x7F17mbzaJUjMbq3ZRjLoxXmmjppfy6FETjPO1bOlH92ld3hl /JZaVQJw/H2NqKN9BzYvLSrkyaU6VGttqog94ugacwp0BJYg6T6TI630tJdBiC3tXxRf1WqO3 jhb1igeqmCSxjXW1TFbW5XwKlNOE3/bcprlizxuZg5QF2zqPpqGaupaGRfUb1p+3FiWh1I9Ae mQUUjX9IdOE1ttA== Received-SPF: none client-ip=212.227.17.13; envelope-from=laurent@vivier.eu; helo=mout.kundenserver.de X-Spam_score_int: -33 X-Spam_score: -3.4 X-Spam_bar: --- X-Spam_report: (-3.4 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-1.473, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Le 09/09/2023 à 11:48, Mark Cave-Ayland a écrit : > The swim chip provides an implementation of both Apple's IWM and ISM floppy disk > controllers. Split the existing implementation into separate register banks for > each controller, whilst also switching the IWM registers from 16-bit to 8-bit > as implemented in real hardware. > > Signed-off-by: Mark Cave-Ayland > --- > hw/block/swim.c | 85 ++++++++++++++++++++++++----------------- > hw/block/trace-events | 4 +- > include/hw/block/swim.h | 15 +++----- > 3 files changed, 58 insertions(+), 46 deletions(-) > > diff --git a/hw/block/swim.c b/hw/block/swim.c > index 7df36ea139..735b335883 100644 > --- a/hw/block/swim.c > +++ b/hw/block/swim.c > @@ -126,7 +126,14 @@ > #define SWIM_HEDSEL 0x20 > #define SWIM_MOTON 0x80 > > -static const char *swim_reg_names[] = { > +static const char *iwm_reg_names[] = { > + "PH0L", "PH0H", "PH1L", "PH1H", > + "PH2L", "PH2H", "PH3L", "PH3H", > + "MTROFF", "MTRON", "INTDRIVE", "EXTDRIVE", > + "Q6L", "Q6H", "Q7L", "Q7H" > +}; > + > +static const char *ism_reg_names[] = { > "WRITE_DATA", "WRITE_MARK", "WRITE_CRC", "WRITE_PARAMETER", > "WRITE_PHASE", "WRITE_SETUP", "WRITE_MODE0", "WRITE_MODE1", > "READ_DATA", "READ_MARK", "READ_ERROR", "READ_PARAMETER", > @@ -274,12 +281,11 @@ static void iwmctrl_write(void *opaque, hwaddr reg, uint64_t value, > > reg >>= REG_SHIFT; > > - swimctrl->regs[reg >> 1] = reg & 1; > - trace_swim_iwmctrl_write((reg >> 1), size, (reg & 1)); > + swimctrl->iwmregs[reg] = value; > + trace_swim_iwmctrl_write(reg, iwm_reg_names[reg], size, value); > > - if (swimctrl->regs[IWM_Q6] && > - swimctrl->regs[IWM_Q7]) { > - if (swimctrl->regs[IWM_MTR]) { > + if (swimctrl->iwmregs[IWM_Q7H]) { > + if (swimctrl->iwmregs[IWM_MTRON]) { > /* data register */ > swimctrl->iwm_data = value; > } else { > @@ -307,6 +313,12 @@ static void iwmctrl_write(void *opaque, hwaddr reg, uint64_t value, > swimctrl->mode = SWIM_MODE_SWIM; > swimctrl->iwm_switch = 0; > trace_swim_iwm_switch(); > + > + /* Switch to ISM registers */ > + memory_region_del_subregion(&swimctrl->swim, > + &swimctrl->iwm); > + memory_region_add_subregion(&swimctrl->swim, 0x0, > + &swimctrl->ism); > } > break; > } > @@ -317,28 +329,30 @@ static void iwmctrl_write(void *opaque, hwaddr reg, uint64_t value, > static uint64_t iwmctrl_read(void *opaque, hwaddr reg, unsigned size) > { > SWIMCtrl *swimctrl = opaque; > + uint16_t value; Why not uint8_t as iwmregs is uint8_t? > > reg >>= REG_SHIFT; > > - swimctrl->regs[reg >> 1] = reg & 1; > + value = swimctrl->iwmregs[reg]; > + trace_swim_iwmctrl_read(reg, iwm_reg_names[reg], size, value); > > - trace_swim_iwmctrl_read((reg >> 1), size, (reg & 1)); > - return 0; > + return value; > } > > -static void swimctrl_write(void *opaque, hwaddr reg, uint64_t value, > - unsigned size) > +static const MemoryRegionOps swimctrl_iwm_ops = { > + .write = iwmctrl_write, > + .read = iwmctrl_read, > + .endianness = DEVICE_BIG_ENDIAN, > +}; > + > +static void ismctrl_write(void *opaque, hwaddr reg, uint64_t value, > + unsigned size) > { > SWIMCtrl *swimctrl = opaque; > > - if (swimctrl->mode == SWIM_MODE_IWM) { > - iwmctrl_write(opaque, reg, value, size); > - return; > - } > - > reg >>= REG_SHIFT; > > - trace_swim_swimctrl_write(reg, swim_reg_names[reg], size, value); > + trace_swim_swimctrl_write(reg, ism_reg_names[reg], size, value); > > switch (reg) { > case SWIM_WRITE_PHASE: > @@ -359,15 +373,11 @@ static void swimctrl_write(void *opaque, hwaddr reg, uint64_t value, > } > } > > -static uint64_t swimctrl_read(void *opaque, hwaddr reg, unsigned size) > +static uint64_t ismctrl_read(void *opaque, hwaddr reg, unsigned size) > { > SWIMCtrl *swimctrl = opaque; > uint32_t value = 0; > > - if (swimctrl->mode == SWIM_MODE_IWM) { > - return iwmctrl_read(opaque, reg, size); > - } > - > reg >>= REG_SHIFT; > > switch (reg) { > @@ -389,14 +399,14 @@ static uint64_t swimctrl_read(void *opaque, hwaddr reg, unsigned size) > break; > } > > - trace_swim_swimctrl_read(reg, swim_reg_names[reg], size, value); > + trace_swim_swimctrl_read(reg, ism_reg_names[reg], size, value); > return value; > } > > -static const MemoryRegionOps swimctrl_mem_ops = { > - .write = swimctrl_write, > - .read = swimctrl_read, > - .endianness = DEVICE_NATIVE_ENDIAN, > +static const MemoryRegionOps swimctrl_ism_ops = { > + .write = ismctrl_write, > + .read = ismctrl_read, > + .endianness = DEVICE_BIG_ENDIAN, > }; > > static void sysbus_swim_reset(DeviceState *d) > @@ -407,13 +417,13 @@ static void sysbus_swim_reset(DeviceState *d) > > ctrl->mode = 0; > ctrl->iwm_switch = 0; > - for (i = 0; i < 8; i++) { > - ctrl->regs[i] = 0; > - } > ctrl->iwm_data = 0; > ctrl->iwm_mode = 0; > + memset(ctrl->iwmregs, 0, 16); > + > ctrl->swim_phase = 0; > ctrl->swim_mode = 0; > + memset(ctrl->ismregs, 0, 16); > for (i = 0; i < SWIM_MAX_FD; i++) { > fd_recalibrate(&ctrl->drives[i]); > } > @@ -425,9 +435,12 @@ static void sysbus_swim_init(Object *obj) > Swim *sbs = SWIM(obj); > SWIMCtrl *swimctrl = &sbs->ctrl; > > - memory_region_init_io(&swimctrl->iomem, obj, &swimctrl_mem_ops, swimctrl, > - "swim", 0x2000); > - sysbus_init_mmio(sbd, &swimctrl->iomem); > + memory_region_init(&swimctrl->swim, obj, "swim", 0x2000); > + memory_region_init_io(&swimctrl->iwm, obj, &swimctrl_iwm_ops, swimctrl, > + "iwm", 0x2000); > + memory_region_init_io(&swimctrl->ism, obj, &swimctrl_ism_ops, swimctrl, > + "ism", 0x2000); > + sysbus_init_mmio(sbd, &swimctrl->swim); > } > > static void sysbus_swim_realize(DeviceState *dev, Error **errp) > @@ -437,6 +450,9 @@ static void sysbus_swim_realize(DeviceState *dev, Error **errp) > > qbus_init(&swimctrl->bus, sizeof(SWIMBus), TYPE_SWIM_BUS, dev, NULL); > swimctrl->bus.ctrl = swimctrl; > + > + /* Default register set is IWM */ > + memory_region_add_subregion(&swimctrl->swim, 0x0, &swimctrl->iwm); > } > > static const VMStateDescription vmstate_fdrive = { > @@ -456,10 +472,11 @@ static const VMStateDescription vmstate_swim = { > VMSTATE_INT32(mode, SWIMCtrl), > /* IWM mode */ > VMSTATE_INT32(iwm_switch, SWIMCtrl), > - VMSTATE_UINT16_ARRAY(regs, SWIMCtrl, 8), > + VMSTATE_UINT8_ARRAY(iwmregs, SWIMCtrl, 16), > VMSTATE_UINT8(iwm_data, SWIMCtrl), > VMSTATE_UINT8(iwm_mode, SWIMCtrl), > /* SWIM mode */ > + VMSTATE_UINT8_ARRAY(ismregs, SWIMCtrl, 16), > VMSTATE_UINT8(swim_phase, SWIMCtrl), > VMSTATE_UINT8(swim_mode, SWIMCtrl), > /* Drives */ > diff --git a/hw/block/trace-events b/hw/block/trace-events > index c041ec45e3..ea84ad6c77 100644 > --- a/hw/block/trace-events > +++ b/hw/block/trace-events > @@ -94,6 +94,6 @@ m25p80_binding_no_bdrv(void *s) "[%p] No BDRV - binding to RAM" > # swim.c > swim_swimctrl_read(int reg, const char *name, unsigned size, uint64_t value) "reg=%d [%s] size=%u value=0x%"PRIx64 > swim_swimctrl_write(int reg, const char *name, unsigned size, uint64_t value) "reg=%d [%s] size=%u value=0x%"PRIx64 > -swim_iwmctrl_read(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64 > -swim_iwmctrl_write(int reg, unsigned size, uint64_t value) "reg=%d size=%u value=0x%"PRIx64 > +swim_iwmctrl_read(int reg, const char *name, unsigned size, uint64_t value) "reg=%d [%s] size=%u value=0x%"PRIx64 > +swim_iwmctrl_write(int reg, const char *name, unsigned size, uint64_t value) "reg=%d [%s] size=%u value=0x%"PRIx64 > swim_iwm_switch(void) "switch from IWM to SWIM mode" > diff --git a/include/hw/block/swim.h b/include/hw/block/swim.h > index 9b3dcb029d..1bc7635d02 100644 > --- a/include/hw/block/swim.h > +++ b/include/hw/block/swim.h > @@ -43,23 +43,18 @@ typedef struct FDrive { > } FDrive; > > struct SWIMCtrl { > - MemoryRegion iomem; > + MemoryRegion swim; > + MemoryRegion iwm; > + MemoryRegion ism; > FDrive drives[SWIM_MAX_FD]; > int mode; > /* IWM mode */ > int iwm_switch; > - uint16_t regs[8]; > -#define IWM_PH0 0 > -#define IWM_PH1 1 > -#define IWM_PH2 2 > -#define IWM_PH3 3 > -#define IWM_MTR 4 > -#define IWM_DRIVE 5 > -#define IWM_Q6 6 > -#define IWM_Q7 7 > + uint8_t iwmregs[16]; > uint8_t iwm_data; > uint8_t iwm_mode; > /* SWIM mode */ > + uint8_t ismregs[16]; > uint8_t swim_phase; > uint8_t swim_mode; > SWIMBus bus; Reviewed-by: Laurent Vivier