All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: "Deucher, Alexander" <Alexander.Deucher@amd.com>
Cc: "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER
	MANAGEM..." <alsa-devel@alsa-project.org>,
	Maruthi Srinivas Bayyavarapu <Maruthi.Bayyavarapu@amd.com>,
	open list <linux-kernel@vger.kernel.org>,
	Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	"Mehta, Sanju" <Sanju.Mehta@amd.com>,
	Mark Brown <broonie@kernel.org>,
	"Mukunda, Vijendar" <Vijendar.Mukunda@amd.com>,
	"RAVULAPATI,
	VISHNU VARDHAN RAO" <Vishnuvardhanrao.Ravulapati@amd.com>,
	Colin Ian King <colin.king@canonical.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [alsa-devel] [PATCH 2/7] ASoC: amd: Registering device endpoints using MFD framework
Date: Wed, 2 Oct 2019 14:35:53 +0100	[thread overview]
Message-ID: <20191002133553.GA21172@dell> (raw)
In-Reply-To: <BN6PR12MB1809451A3152488F3D8D1371F79C0@BN6PR12MB1809.namprd12.prod.outlook.com>

On Wed, 02 Oct 2019, Deucher, Alexander wrote:

> > -----Original Message-----
> > From: Lee Jones <lee.jones@linaro.org>
> > Sent: Wednesday, October 2, 2019 8:38 AM
> > To: Deucher, Alexander <Alexander.Deucher@amd.com>
> > Cc: RAVULAPATI, VISHNU VARDHAN RAO
> > <Vishnuvardhanrao.Ravulapati@amd.com>; Liam Girdwood
> > <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; Jaroslav
> > Kysela <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Mukunda,
> > Vijendar <Vijendar.Mukunda@amd.com>; Maruthi Srinivas Bayyavarapu
> > <Maruthi.Bayyavarapu@amd.com>; Mehta, Sanju
> > <Sanju.Mehta@amd.com>; Colin Ian King <colin.king@canonical.com>; Dan
> > Carpenter <dan.carpenter@oracle.com>; moderated list:SOUND - SOC LAYER
> > / DYNAMIC AUDIO POWER MANAGEM... <alsa-devel@alsa-project.org>;
> > open list <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH 2/7] ASoC: amd: Registering device endpoints using MFD
> > framework
> > 
> > On Tue, 01 Oct 2019, Deucher, Alexander wrote:
> > 
> > > > -----Original Message-----
> > > > From: Lee Jones <lee.jones@linaro.org>
> > > > Sent: Tuesday, October 1, 2019 8:00 AM
> > > > To: RAVULAPATI, VISHNU VARDHAN RAO
> > > > <Vishnuvardhanrao.Ravulapati@amd.com>
> > > > Cc: RAVULAPATI, VISHNU VARDHAN RAO
> > > > <Vishnuvardhanrao.Ravulapati@amd.com>; Deucher, Alexander
> > > > <Alexander.Deucher@amd.com>; Liam Girdwood
> > <lgirdwood@gmail.com>;
> > > > Mark Brown <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>;
> > > > Takashi Iwai <tiwai@suse.com>; Mukunda, Vijendar
> > > > <Vijendar.Mukunda@amd.com>; Maruthi Srinivas Bayyavarapu
> > > > <Maruthi.Bayyavarapu@amd.com>; Mehta, Sanju
> > <Sanju.Mehta@amd.com>;
> > > > Colin Ian King <colin.king@canonical.com>; Dan Carpenter
> > > > <dan.carpenter@oracle.com>; moderated list:SOUND - SOC LAYER /
> > > > DYNAMIC AUDIO POWER MANAGEM... <alsa-devel@alsa-project.org>;
> > open
> > > > list <linux-kernel@vger.kernel.org>
> > > > Subject: Re: [PATCH 2/7] ASoC: amd: Registering device endpoints
> > > > using MFD framework
> > > >
> > > > On Tue, 01 Oct 2019, vishnu wrote:
> > > >
> > > > > Hi Jones,
> > > > >
> > > > > I am very Thankful to your review comments.
> > > > >
> > > > > Actually The driver is not totally based on MFD. It just uses
> > > > > mfd_add_hotplug_devices() and mfd_remove_devices() for adding
> > the
> > > > > devices automatically.
> > > > >
> > > > > Remaining code has nothing to do with MFD framework.
> > > > >
> > > > > So I thought It would not break the coding style and moved ahead
> > > > > by using the MFD API by adding its header file.
> > > > >
> > > > > If it is any violation of coding standard then I can move it to
> > > > > drivers/mfd.
> > > > >
> > > > > This patch could be a show stopper for us.Please suggest us how
> > > > > can we move ahead ASAP.
> > > >
> > > > Either move the MFD parts to drivers/mfd, or stop using the MFD API.
> > >
> > > There are more drivers outside of drivers/mfd using this API than
> > > drivers in drivers/mfd.
> > 
> > People do wrong things all the time.  It doesn't make them right.
> > 
> > > In a lot of cases it doesn't make sense to move the driver to drivers/mfd.
> > 
> > In those cases, the platform_device_*() API should be used.
> 
> Why do we have both?  It's not clear to me on when we should use one

The platform_device_*() API is the de facto API to use for registering
devices.  mfd_*() is a framework built on-top of that for devices
which register sub-devices that do not reasonably reside elsewhere.

The mfd_*() helper functions should only be used by MFD devices.

> vs the other.  These are not platforms per se, they are PCI devices
> that happen to have other devices on them.  On previous projects, I
> was told to use mfd and no objections were raised at that time.

Who told you to use MFD API outside of drivers/mfd?  That's a hack.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: "Deucher, Alexander" <Alexander.Deucher@amd.com>
Cc: "RAVULAPATI,
	VISHNU VARDHAN RAO"  <Vishnuvardhanrao.Ravulapati@amd.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	"Mukunda, Vijendar" <Vijendar.Mukunda@amd.com>,
	Maruthi Srinivas Bayyavarapu <Maruthi.Bayyavarapu@amd.com>,
	"Mehta, Sanju" <Sanju.Mehta@amd.com>,
	Colin Ian King <colin.king@canonical.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	"moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER
	MANAGEM..."  <alsa-devel@alsa-project.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/7] ASoC: amd: Registering device endpoints using MFD framework
Date: Wed, 2 Oct 2019 14:35:53 +0100	[thread overview]
Message-ID: <20191002133553.GA21172@dell> (raw)
In-Reply-To: <BN6PR12MB1809451A3152488F3D8D1371F79C0@BN6PR12MB1809.namprd12.prod.outlook.com>

On Wed, 02 Oct 2019, Deucher, Alexander wrote:

> > -----Original Message-----
> > From: Lee Jones <lee.jones@linaro.org>
> > Sent: Wednesday, October 2, 2019 8:38 AM
> > To: Deucher, Alexander <Alexander.Deucher@amd.com>
> > Cc: RAVULAPATI, VISHNU VARDHAN RAO
> > <Vishnuvardhanrao.Ravulapati@amd.com>; Liam Girdwood
> > <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; Jaroslav
> > Kysela <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Mukunda,
> > Vijendar <Vijendar.Mukunda@amd.com>; Maruthi Srinivas Bayyavarapu
> > <Maruthi.Bayyavarapu@amd.com>; Mehta, Sanju
> > <Sanju.Mehta@amd.com>; Colin Ian King <colin.king@canonical.com>; Dan
> > Carpenter <dan.carpenter@oracle.com>; moderated list:SOUND - SOC LAYER
> > / DYNAMIC AUDIO POWER MANAGEM... <alsa-devel@alsa-project.org>;
> > open list <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH 2/7] ASoC: amd: Registering device endpoints using MFD
> > framework
> > 
> > On Tue, 01 Oct 2019, Deucher, Alexander wrote:
> > 
> > > > -----Original Message-----
> > > > From: Lee Jones <lee.jones@linaro.org>
> > > > Sent: Tuesday, October 1, 2019 8:00 AM
> > > > To: RAVULAPATI, VISHNU VARDHAN RAO
> > > > <Vishnuvardhanrao.Ravulapati@amd.com>
> > > > Cc: RAVULAPATI, VISHNU VARDHAN RAO
> > > > <Vishnuvardhanrao.Ravulapati@amd.com>; Deucher, Alexander
> > > > <Alexander.Deucher@amd.com>; Liam Girdwood
> > <lgirdwood@gmail.com>;
> > > > Mark Brown <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>;
> > > > Takashi Iwai <tiwai@suse.com>; Mukunda, Vijendar
> > > > <Vijendar.Mukunda@amd.com>; Maruthi Srinivas Bayyavarapu
> > > > <Maruthi.Bayyavarapu@amd.com>; Mehta, Sanju
> > <Sanju.Mehta@amd.com>;
> > > > Colin Ian King <colin.king@canonical.com>; Dan Carpenter
> > > > <dan.carpenter@oracle.com>; moderated list:SOUND - SOC LAYER /
> > > > DYNAMIC AUDIO POWER MANAGEM... <alsa-devel@alsa-project.org>;
> > open
> > > > list <linux-kernel@vger.kernel.org>
> > > > Subject: Re: [PATCH 2/7] ASoC: amd: Registering device endpoints
> > > > using MFD framework
> > > >
> > > > On Tue, 01 Oct 2019, vishnu wrote:
> > > >
> > > > > Hi Jones,
> > > > >
> > > > > I am very Thankful to your review comments.
> > > > >
> > > > > Actually The driver is not totally based on MFD. It just uses
> > > > > mfd_add_hotplug_devices() and mfd_remove_devices() for adding
> > the
> > > > > devices automatically.
> > > > >
> > > > > Remaining code has nothing to do with MFD framework.
> > > > >
> > > > > So I thought It would not break the coding style and moved ahead
> > > > > by using the MFD API by adding its header file.
> > > > >
> > > > > If it is any violation of coding standard then I can move it to
> > > > > drivers/mfd.
> > > > >
> > > > > This patch could be a show stopper for us.Please suggest us how
> > > > > can we move ahead ASAP.
> > > >
> > > > Either move the MFD parts to drivers/mfd, or stop using the MFD API.
> > >
> > > There are more drivers outside of drivers/mfd using this API than
> > > drivers in drivers/mfd.
> > 
> > People do wrong things all the time.  It doesn't make them right.
> > 
> > > In a lot of cases it doesn't make sense to move the driver to drivers/mfd.
> > 
> > In those cases, the platform_device_*() API should be used.
> 
> Why do we have both?  It's not clear to me on when we should use one

The platform_device_*() API is the de facto API to use for registering
devices.  mfd_*() is a framework built on-top of that for devices
which register sub-devices that do not reasonably reside elsewhere.

The mfd_*() helper functions should only be used by MFD devices.

> vs the other.  These are not platforms per se, they are PCI devices
> that happen to have other devices on them.  On previous projects, I
> was told to use mfd and no objections were raised at that time.

Who told you to use MFD API outside of drivers/mfd?  That's a hack.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

  reply	other threads:[~2019-10-02 13:36 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-01  0:58 [alsa-devel] [PATCH 1/7] ASoC: amd: No need PCI-MSI interrupts Ravulapati Vishnu vardhan rao
2019-10-01  0:58 ` Ravulapati Vishnu vardhan rao
2019-10-01  0:58 ` [alsa-devel] [PATCH 2/7] ASoC: amd: Registering device endpoints using MFD framework Ravulapati Vishnu vardhan rao
2019-10-01  0:58   ` Ravulapati Vishnu vardhan rao
2019-10-01  6:45   ` [alsa-devel] " Lee Jones
2019-10-01 10:00     ` vishnu
2019-10-01 10:00       ` vishnu
2019-10-01 12:00       ` [alsa-devel] " Lee Jones
2019-10-01 12:00         ` Lee Jones
2019-10-01 18:53         ` [alsa-devel] " Deucher, Alexander
2019-10-01 18:53           ` Deucher, Alexander
2019-10-02 12:37           ` [alsa-devel] " Lee Jones
2019-10-02 12:37             ` Lee Jones
2019-10-02 13:11             ` [alsa-devel] " Deucher, Alexander
2019-10-02 13:11               ` Deucher, Alexander
2019-10-02 13:35               ` Lee Jones [this message]
2019-10-02 13:35                 ` Lee Jones
2019-10-10 13:08                 ` [alsa-devel] " RAVULAPATI, VISHNU VARDHAN RAO
2019-10-14  7:03                   ` Lee Jones
2019-10-14  7:03                     ` Lee Jones
2019-10-17  9:26                     ` [alsa-devel] " vishnu
2019-10-17  9:26                       ` vishnu
2019-10-01  0:58 ` [alsa-devel] [PATCH 3/7] ASoC: amd: Refactoring of DAI from DMA driver Ravulapati Vishnu vardhan rao
2019-10-01  0:58   ` Ravulapati Vishnu vardhan rao
2019-10-01  0:58 ` [alsa-devel] [PATCH 4/7] ASoC: amd: Enabling I2S instance in DMA and DAI Ravulapati Vishnu vardhan rao
2019-10-01  0:58   ` Ravulapati Vishnu vardhan rao
2019-10-01  0:58 ` [alsa-devel] [PATCH 5/7] ASoC: amd: add ACP3x TDM mode support Ravulapati Vishnu vardhan rao
2019-10-01  0:58   ` Ravulapati Vishnu vardhan rao
2019-10-01  0:58 ` [alsa-devel] [PATCH 6/7] ASoC: AMD: handle ACP3x i2s-sp watermark interrupt Ravulapati Vishnu vardhan rao
2019-10-01  0:58   ` Ravulapati Vishnu vardhan rao
2019-10-01  0:58 ` [alsa-devel] [PATCH 7/7] ASoc: amd: Added ACP3x system resume and runtime pm ops Ravulapati Vishnu vardhan rao
2019-10-01  0:58   ` Ravulapati Vishnu vardhan rao
2019-10-01 17:23 ` [alsa-devel] [PATCH 1/7] ASoC: amd: No need PCI-MSI interrupts Deucher, Alexander
2019-10-01 17:23   ` Deucher, Alexander
2019-10-01 17:29   ` [alsa-devel] " Mark Brown
2019-10-01 17:29     ` Mark Brown
2019-10-10 10:40     ` [alsa-devel] " vishnu
2019-10-10 10:40       ` vishnu
2019-10-17  9:33       ` [alsa-devel] " vishnu
2019-10-17  9:33         ` vishnu
2019-10-17 11:12         ` [alsa-devel] " Mark Brown
2019-10-17 11:12           ` Mark Brown
2019-10-17 13:56         ` [alsa-devel] " Deucher, Alexander
2019-10-17 13:56           ` Deucher, Alexander
2019-10-17  9:31     ` [alsa-devel] " vishnu
2019-10-17  9:31       ` vishnu

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=20191002133553.GA21172@dell \
    --to=lee.jones@linaro.org \
    --cc=Alexander.Deucher@amd.com \
    --cc=Maruthi.Bayyavarapu@amd.com \
    --cc=Sanju.Mehta@amd.com \
    --cc=Vijendar.Mukunda@amd.com \
    --cc=Vishnuvardhanrao.Ravulapati@amd.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=colin.king@canonical.com \
    --cc=dan.carpenter@oracle.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tiwai@suse.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 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.