All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Richardson <jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
To: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andy Shevchenko
	<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Dmitry Torokhov <dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Anatol Pomazau <anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	"Kumar Gala" <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	<linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	bcm-kernel-feedback-list
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Rafal Milecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 3/4] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips
Date: Mon, 6 Apr 2015 11:39:03 -0700	[thread overview]
Message-ID: <5522D2C7.8090708@broadcom.com> (raw)
In-Reply-To: <551ED345.1000702-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 15-04-03 10:52 AM, Florian Fainelli wrote:
> On 03/04/15 06:38, Andy Shevchenko wrote:
>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>> <jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> 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.
> 

I cringed adding the ifdefs to the driver but didn't think the small
amount of code that wouldn't be used again warranted 3 files. I could
also handle the two different probe routines by doing some DT parsing in
init, but then BCMA would have to be compiled for the non-BCMA MSPI
driver and I didn't want to do that either.



--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Richardson <jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
To: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andy Shevchenko
	<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Dmitry Torokhov <dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Anatol Pomazau <anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bcm-kernel-feedback-list
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Rafal Milecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 3/4] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips
Date: Mon, 6 Apr 2015 11:39:03 -0700	[thread overview]
Message-ID: <5522D2C7.8090708@broadcom.com> (raw)
In-Reply-To: <551ED345.1000702-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 15-04-03 10:52 AM, Florian Fainelli wrote:
> On 03/04/15 06:38, Andy Shevchenko wrote:
>> On Thu, Apr 2, 2015 at 10:23 PM, Jonathan Richardson
>> <jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> 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.
> 

I cringed adding the ifdefs to the driver but didn't think the small
amount of code that wouldn't be used again warranted 3 files. I could
also handle the two different probe routines by doing some DT parsing in
init, but then BCMA would have to be compiled for the non-BCMA MSPI
driver and I didn't want to do that either.



--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Richardson <jonathar@broadcom.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	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: Mon, 6 Apr 2015 11:39:03 -0700	[thread overview]
Message-ID: <5522D2C7.8090708@broadcom.com> (raw)
In-Reply-To: <551ED345.1000702@gmail.com>

On 15-04-03 10:52 AM, Florian Fainelli wrote:
> 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.
> 

I cringed adding the ifdefs to the driver but didn't think the small
amount of code that wouldn't be used again warranted 3 files. I could
also handle the two different probe routines by doing some DT parsing in
init, but then BCMA would have to be compiled for the non-BCMA MSPI
driver and I didn't want to do that either.




  parent reply	other threads:[~2015-04-06 18:39 UTC|newest]

Thread overview: 47+ 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 ` Jonathan Richardson
2015-04-02 19:23 ` Jonathan Richardson
     [not found] ` <1428002603-21892-1-git-send-email-jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-02 19:23   ` [PATCH 1/4] ARM: dts: Add binding for Broadcom MSPI driver Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-04 19:17     ` Florian Fainelli
2015-04-06 18:45       ` Jonathan Richardson
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-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-03 13:35     ` Andy Shevchenko
     [not found]       ` <CAHp75Vf0JrB5pqNEouLzyOf5mmi05Ut_cybd37suTrLbJOFHKA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-06 10:18         ` Rafał Miłecki
2015-04-06 10:18           ` Rafał Miłecki
2015-04-06 18:58           ` Jonathan Richardson
2015-04-06 18:30       ` Jonathan Richardson
2015-04-06 18:30         ` Jonathan Richardson
     [not found]         ` <5522D0BA.2050805-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-07  8:03           ` Andy Shevchenko
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-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
     [not found]     ` <1428002603-21892-4-git-send-email-jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-03 13:38       ` Andy Shevchenko
2015-04-03 13:38         ` Andy Shevchenko
     [not found]         ` <CAHp75VcMg-8X8DtHbQrmugzWG5VzzHXcpWNiyrTT=qmuCvA95w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-03 17:52           ` Florian Fainelli
2015-04-03 17:52             ` Florian Fainelli
     [not found]             ` <551ED345.1000702-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-06 10:36               ` Rafał Miłecki
2015-04-06 10:36                 ` Rafał Miłecki
2015-04-06 19:09                 ` Jonathan Richardson
2015-04-06 18:39               ` Jonathan Richardson [this message]
2015-04-06 18:39                 ` 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-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-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
     [not found]     ` <1428002603-21892-5-git-send-email-jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-04 19:12       ` Florian Fainelli
2015-04-04 19:12         ` Florian Fainelli
     [not found]         ` <552037BB.7050803-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-06  9:46           ` Mark Brown
2015-04-06  9:46             ` Mark Brown
     [not found]             ` <20150406094636.GC6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-04-06 18:54               ` Jonathan Richardson
2015-04-06 18:54                 ` Jonathan Richardson
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=5522D2C7.8090708@broadcom.com \
    --to=jonathar-dy08kvg/lbpwk0htik3j/w@public.gmane.org \
    --cc=anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.