linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Jonathan Richardson <jonathar@broadcom.com>
Cc: Mark Brown <broonie@kernel.org>,
	Dmitry Torokhov <dtor@google.com>,
	Anatol Pomazau <anatol@google.com>,
	Scott Branden <sbranden@broadcom.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-spi@vger.kernel.org,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	devicetree <devicetree@vger.kernel.org>,
	Rafal Milecki <zajec5@gmail.com>
Subject: Re: [PATCH 3/4] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips
Date: Fri, 03 Apr 2015 10:52:05 -0700	[thread overview]
Message-ID: <551ED345.1000702@gmail.com> (raw)
In-Reply-To: <CAHp75VcMg-8X8DtHbQrmugzWG5VzzHXcpWNiyrTT=qmuCvA95w@mail.gmail.com>

On 03/04/15 06:38, Andy Shevchenko wrote:
> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
> <jonathar@broadcom.com> 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?

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.
-- 
Florian

  reply	other threads:[~2015-04-03 17:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-02 19:23 [PATCH 0/4] Add MSPI support for Cygnus Jonathan Richardson
2015-04-02 19:23 ` [PATCH 1/4] ARM: dts: Add binding for Broadcom MSPI driver Jonathan Richardson
2015-04-04 19:17   ` Florian Fainelli
2015-04-06 18:45     ` Jonathan Richardson
2015-04-02 19:23 ` [PATCH 2/4] spi: bcm53xx: Refactor to make driver nonspecific to 53xx SoCs Jonathan Richardson
2015-04-03 13:35   ` Andy Shevchenko
2015-04-06 10:18     ` Rafał Miłecki
2015-04-06 18:58       ` Jonathan Richardson
2015-04-06 18:30     ` Jonathan Richardson
2015-04-07  8:03       ` Andy Shevchenko
2015-04-02 19:23 ` [PATCH 3/4] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips Jonathan Richardson
2015-04-03 13:38   ` Andy Shevchenko
2015-04-03 17:52     ` Florian Fainelli [this message]
2015-04-06 10:36       ` Rafał Miłecki
2015-04-06 19:09         ` Jonathan Richardson
2015-04-06 18:39       ` Jonathan Richardson
2015-04-06 10:26     ` Rafał Miłecki
2015-04-06 10:26   ` Rafał Miłecki
2015-04-02 19:23 ` [PATCH 4/4] spi: bcm-mspi: Add support to set serial baud clock rate Jonathan Richardson
2015-04-04 19:12   ` Florian Fainelli
2015-04-06  9:46     ` Mark Brown
2015-04-06 18:54       ` Jonathan Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=551ED345.1000702@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=anatol@google.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dtor@google.com \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jonathar@broadcom.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sbranden@broadcom.com \
    --cc=zajec5@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).