From patchwork Tue Jun 5 01:54:04 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Peter A. G. Crosthwaite" X-Patchwork-Id: 162911 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 9239BB6F9F for ; Tue, 5 Jun 2012 11:54:22 +1000 (EST) Received: from localhost ([::1]:35353 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbiyS-0002hR-I1 for incoming@patchwork.ozlabs.org; Mon, 04 Jun 2012 21:54:20 -0400 Received: from eggs.gnu.org ([208.118.235.92]:42171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbiyI-0002PU-Hz for qemu-devel@nongnu.org; Mon, 04 Jun 2012 21:54:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SbiyF-0006dR-H6 for qemu-devel@nongnu.org; Mon, 04 Jun 2012 21:54:10 -0400 Received: from mail-bk0-f45.google.com ([209.85.214.45]:59945) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbiyF-0006cq-9w for qemu-devel@nongnu.org; Mon, 04 Jun 2012 21:54:07 -0400 Received: by bkwj10 with SMTP id j10so4686640bkw.4 for ; Mon, 04 Jun 2012 18:54:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding:x-gm-message-state; bh=OvuC8yfDjTXqWZJgNGXE3NSHIILd2P2MuATQvsG1lUc=; b=T8scdhhQLRxvvormn1GL6nnmbQxt+OCTz5+58WzXRRcmBxQfMf/PeqnszKwJMAZ4Je G7G1vV3GUS/aPwW1dLZMzUygh4DFLsL9UljPv6Og61G8qdtyE7t06pI1iNmEiayj5ZJ0 PpNOgEqm6pi9em/EPTq8hhXIfDVm/CbBLoZvTgsUXaQT5L/bh1k5ZeWI3+4rwuKLVMU2 RyhTIgNpRORiL65vC62aa6/K8uCS9MPdZUxWkxKzigm2IxmBMoQvEdiVubzEO+jwjHjG LwmSf2g9WBMGpmfIsp1EOYw/tv7GF80wDy/2a/5RrpfmdyJ7/xCIJQMTdVNBcTfXS78w RzqQ== MIME-Version: 1.0 Received: by 10.205.137.5 with SMTP id im5mr8221995bkc.45.1338861244597; Mon, 04 Jun 2012 18:54:04 -0700 (PDT) Received: by 10.205.81.68 with HTTP; Mon, 4 Jun 2012 18:54:04 -0700 (PDT) In-Reply-To: <201206050234.09903.paul@codesourcery.com> References: <201206050234.09903.paul@codesourcery.com> Date: Tue, 5 Jun 2012 11:54:04 +1000 Message-ID: From: Peter Crosthwaite To: Paul Brook X-Gm-Message-State: ALoCoQm6YOvDoa88IzvI1ABZEDDKdf2vvRqvc5J4zgarbnozOoDY+q35nU5PDYHipZa02eQReyfv X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.214.45 Cc: stefanha@gmail.com, edgar.iglesias@gmail.com, qemu-devel@nongnu.org, john.williams@petalogix.com, peter.maydell@linaro.org Subject: Re: [Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Hi Paul, Responses inline below. On Tue, Jun 5, 2012 at 11:34 AM, Paul Brook wrote: >> Patch 1 Enhances SSI bus support to properly support multiple attached >> devices. An api is provided for SSI/SPI masters to select a particular >> device attached to the bus. >> >> Patch 2 is a device model for the m25p80 style SPI flash chip. >> >> Patch 3 is  the Xilinx XPS SPI contoller. Its a sysbus device that >> instantiates a ssi bus, and interfaces the two (as per the controllers >> functionality) >> >> Patch 4 instantiates the XPS SPI controller in the petalogix ML605 >> reference platform and connects two m25p80s to it. >> >> Patch 5 updates the stellaris machine model to use the multi slave SSI >> support > > I'm still not convinced modelling this as a multipoint bus is a good idea.  If > nothing else you've failed to model the case where multiple slaves are > selected simultanously. The bus can easily be changed such that multiple devices are selectable at once to get your desired multi device behaviour. AFAICT though nothing in QEMU behaves like this ATM.  Given the chip selects are actual wires, not part of > the bus itself, I think multiple point-point busses are a better fit. > > For the stellaris device we still have the synthetic mux device and > intermediate bus. > Yes, because in your stellaris architecture, the SSI controller (pl022) is point to point so that exactly matches the hardware. In the microblaze controller in this series, the controller has inbuilt muxing with one-hot CS behavior. To implement with point to point, I would have to dynamically create a number of sub-busses (driven by a qdev property). I would also have to have a device within a device to model the internal mux which increases my code volume significantly. Also you end up with this little piece of ugliness in your machine model and device model: The multi-slave bus is a direct superset on point-to-point. There is nothing stopping anyone from using it as p2p. Its just things are very ugly for SPI controllers with integrated muxes to treat everything as point to point. Regards, Peter > > Paul diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c index 01af0da..394a80e 100644 --- a/hw/petalogix_ml605_mmu.c +++ b/hw/petalogix_ml605_mmu.c @@ -145,10 +145,11 @@ petalogix_ml605_init(ram_addr_t ram_size, sysbus_mmio_map(busdev, 0, 0x40a00000); sysbus_connect_irq(busdev, 0, irq[4]); - spi = qdev_get_child_bus(dev, "spi"); for (i = 0; i < NUM_SPI_FLASHES; i++) { - dev = ssi_create_slave_no_init(spi, "m25p80", i); + sprintf(sub_bus_name, "spi%d", i); + spi = qdev_get_child_bus(dev, sub_bus_name); + dev = ssi_create_slave_no_init(spi, "m25p80"); qdev_prop_set_string(dev, "partname", (char *)"s25fl064k"); qdev_init_nofail(dev); } diff --git a/hw/xilinx_spi.c b/hw/xilinx_spi.c index cae88ad..6c81f70 100644 --- a/hw/xilinx_spi.c +++ b/hw/xilinx_spi.c @@ -420,8 +420,11 @@ static int xilinx_spi_init(SysBusDevice *dev) s->irqline = -1; ptimer_set_freq(s->ptimer, 10 * 1000 * 1000); - s->spi = ssi_create_bus(&dev->qdev, "spi"); - + char sub_bus_name; + for (i = 0; i < num_cs; i++) { + sprintf(sub_bus_name, "spi%d", i); + s->spi[i] = ssi_create_bus(&dev->qdev, sub_bus_name); + } return 0; }