From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752999AbbDFKgR (ORCPT ); Mon, 6 Apr 2015 06:36:17 -0400 Received: from mail-ie0-f169.google.com ([209.85.223.169]:34577 "EHLO mail-ie0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752654AbbDFKgM convert rfc822-to-8bit (ORCPT ); Mon, 6 Apr 2015 06:36:12 -0400 MIME-Version: 1.0 In-Reply-To: <551ED345.1000702@gmail.com> References: <1428002603-21892-1-git-send-email-jonathar@broadcom.com> <1428002603-21892-4-git-send-email-jonathar@broadcom.com> <551ED345.1000702@gmail.com> Date: Mon, 6 Apr 2015 12:36:11 +0200 Message-ID: Subject: Re: [PATCH 3/4] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= To: Florian Fainelli Cc: Andy Shevchenko , Jonathan Richardson , Mark Brown , Dmitry Torokhov , Anatol Pomazau , Scott Branden , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , "linux-kernel@vger.kernel.org" , linux-spi , bcm-kernel-feedback-list , devicetree Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 3 April 2015 at 19:52, Florian Fainelli wrote: > On 03/04/15 06:38, Andy Shevchenko wrote: >> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson >> wrote: >>> The Broadcom MSPI controller is used on various chips. The driver only >>> supported BCM53xx chips with BCMA (an AMBA bus variant). The driver is >>> refactored to make BCMA optional and provides a new config for non BCMA >>> systems. >> >>> struct bcm_mspi { >>> + #ifdef CONFIG_SPI_BCMA_MSPI >>> struct bcma_device *core; >>> - struct spi_master *master; >>> + #endif >>> >>> + void __iomem *base; >>> + struct spi_master *master; >>> size_t read_offset; >> >>> + void (*mspi_write)(struct bcm_mspi *mspi, u16 offset, u32 value); >>> + u32 (*mspi_read)(struct bcm_mspi *mspi, u16 offset); >>> +}; >> >> To avoid ugly ifdefs I think better to split driver to core part and >> the actual driver part, at the end you will have something like >> mspi-core.c mspi-53xx.c mspi-whatever.c. Check for example spi-dw*.c > > Actually, I am really curious whether we need the special BCMA I/O > accessors in the first place, cannot we just access the MSPI core on > BCM53xx chips using regular MMIO? That would probably solve the > "problem" entirely. Rafal, did you try this before? It's a matter of choice between: 1) Using one design for all bcma users 2) Using one design for all bcm-mspi users I believe no matter which one you choose, you'll break another one. If you take a look at drivers/bcma/host_soc.c, you'll see we've there core->io_addr. I guess you could use it as the base in bcm-mspi. That of course will make you a bit less compatible with other bcma drivers (skipping bcma R/W layer). > As for splitting the driver into a "library" driver which is mostly > independent from the bus and a bus-specific wrapper, I think BCMA is > really the only special case here, which is why I suggested earlier to > Jonathan that we might just prefer ifdefing things out instead of > creating a separate layer just for BCMA. I think you may be right, this #if for bcma shouldn't be that bad and it shouldn't grow in the future. Still, I'd like to get this patch split nicely to review independent changes. -- Rafał