Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 May 2023 16:55:18 -0500
From:   Jeff LaBundy <jeff@labundy.com>
To:     Fred Treven <Fred.Treven@cirrus.com>
Cc:     Ben Bright <Ben.Bright@cirrus.com>,
        James Ogletree <James.Ogletree@cirrus.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Simon Trimmer <simont@opensource.cirrus.com>,
        Charles Keepax <ckeepax@opensource.cirrus.com>,
        Richard Fitzgerald <rf@opensource.cirrus.com>,
        "patches@opensource.cirrus.com" <patches@opensource.cirrus.com>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "lee@kernel.org" <lee@kernel.org>
Subject: Re: [PATCH RFC 5/5] mfd: cs40l26: Add CODEC driver component
Message-ID: <ZHEqxraGB2hZgh1b@nixie71>
References: <1685059471-9598-1-git-send-email-fred.treven@cirrus.com>
 <1685059471-9598-5-git-send-email-fred.treven@cirrus.com>
 <ZHEL30a/YEia7kjr@nixie71>
 <F6FA6CFA-BFC5-47DA-83D1-6330E66195F5@cirrus.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <F6FA6CFA-BFC5-47DA-83D1-6330E66195F5@cirrus.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZU9RaGs3bHV0MWE3RGxMY3F5Ym4yM2graEZNOGgrTE5GMEphUWRiRW8rREx5?=
 =?utf-8?B?T3g4MGxYWllzbkRtc0V3M2N5a3lkUWNTN3owbi9GNzNTeUEwNVJkT20rTGdR?=
 =?utf-8?B?RVhuVzg4Z3RYZzc5bVhzSHRXaUgzSUd6Z1pXTXRHYkVLZEZhVkVwbnhVVUhs?=
 =?utf-8?B?czEwWDdoWFhQaDJmcWpJZGhNWXU1UUp0REJ5MmhGRjdCSXg4RDZ1bkNHVGE0?=
 =?utf-8?B?cnJsNkNGeHN1SEtOaTJBeXIzK1JDYWZEcnl0by83L0ZrTGpGd1JlL0RXY1J6?=
 =?utf-8?B?bytTOGhWdmNPKzlYSy93N2JEaEtSb1RRanZyTzRXQ09pVkpwTHJpVnVvLzVI?=
 =?utf-8?B?VUI5bkQ4WjJyWWVlaU5YNFIrbmxsUmcvdEJMemJsaUM5dEpuRnVLQjUrMDNw?=
 =?utf-8?B?SnRLUjYzd2VHcVU5b2hXaXJxc01CeDd6R3pnYUI0RXN6T2tSY3k4ZkdDZDZQ?=
 =?utf-8?B?UkUrZ0UycCs4YlNQMGxiSGZXQ1JFTDRjd0tscWRjQUN1SU9HSGlER25lbWE4?=
 =?utf-8?B?UFRvRGRkVzdtKzdKRHBQV2VZcjQ2M0ZMOUQreisyUFNnUGhwTlFpK1B6Um8w?=
 =?utf-8?B?UGYwT0s4bDN3aVNqbDBFeUZ2THcwUFEvNERHbVYzT29aYWFVeHdPVXoxWmRH?=
 =?utf-8?B?bEZwWlpHZTBuWjVnWU56V1hYK0pqcjV6dXE2bVFxRTJRdlJkWDY1bjN1Z1hD?=
 =?utf-8?B?M1FNbTlINTNPU2NDMUZ6WWxlMWlKNERPak1PWE9CM2JDYXUxeGhJU0JQaDJz?=
 =?utf-8?B?NHdNSWh4bzQ2aHBvWGZlaWt0U2VyQ01OT2xTbmpFcHBXeXpEUkZsOVhGa2Iz?=
 =?utf-8?B?SmZGYk9OT3VaN0Y4WEJTM2pCMWZjOGVYUWZaaE1wYk43WnY1UnExbUJSL3Vt?=
 =?utf-8?B?UVduT3FlM1JENC9sbFFFZDVQYWRGWWswQ1Q4Q3NKVjFDVitKazk1TGhGdUR3?=
 =?utf-8?B?YVdNaVMvNnpqUTIxOFc3OFcvSy9DZ1Z3dERrNHo4MEVKTTlwMFdTNXdKM05I?=
 =?utf-8?B?KzdMaXNlemU2eGtmTmo0c2FaaW9pcEdTT21waElsZTBuRkVyd3l0Y1VJenZ4?=
 =?utf-8?B?Vzh4ZTk1MisvdWJYSEdZQXJHYzNjcHFUZEJRUkJSTVFFQTdneWtlYTdIUnI1?=
 =?utf-8?B?TjRuczFBWmMyanZvUVVQMzlCdVZhUnNuWmdRRTl6R2czMmRjU1RCdUZlVjhw?=
 =?utf-8?B?aERFN2lpQ0hxWGg1Rjh1QzU1Ti9TWkVOK1R2Q1g0MUJqWkx4VlVWUVVsZ3lK?=
 =?utf-8?B?MDFwbTZTbC8wNmNwclliSXh0ODBIU1pEQlVWZVV6V1BWL05MRDBNUFN6eENj?=
 =?utf-8?B?UnJpRjI0RWxmYW5mZEcxTWJzODlTSU1OTld4SUFBTWdKL3pUR25vcUR5Y1dW?=
 =?utf-8?B?SStoS0JqQ3kyZ3FKZWhNNUl3d1JIdXhmcml6enJ6YWJnbEZrTmpySkYxbkp0?=
 =?utf-8?B?eVNDUHRWUXVua0l5dXhZdzJBRysrTW41bWJPb0ZJMVZvTFFUWXRHKzd0RVZh?=
 =?utf-8?B?d0NhMnIyOGF4c2Y3V25hTDREWEE2TmU2VFpUNHdVZEFHUmFISm5KazVwazBZ?=
 =?utf-8?B?VDZtL2RsdUNqb2ZNUGJ5cDlOSUlLTDBMVldYM0trOExaVmt1bzEwYk00TFZV?=
 =?utf-8?B?Z0EwUENkKzNyeitvczJzQ1k5eGVZOFBYSTRUZzF5TXNVSHpCdEhvT1RvNktQ?=
 =?utf-8?B?M2dha3NoVGk2ZmMvd1IzVUExKzdOZGZWb2paRkNxNlNvbDNsQVZpZkdrczdh?=
 =?utf-8?B?U21EcmlVNDY3QndrWEtnR1lscGZLTEs2T0RYcFd0L1NGRVdadFU2bXZuMEoy?=
 =?utf-8?B?dXVZNVZDWlp3d2JtMjJpbFVycjFLREdnNnRYQXBGTXo4VXFXNmt1UUFiYTdJ?=
 =?utf-8?B?NUlvcDR2d2VOWi9neVRrT21Ddi9VdVdxRVhxeS8xZlpPZXhDZ3grSWFrdEkv?=
 =?utf-8?B?b3l4eWZDZmRldkkwNEJBb3Z0SDhJcmFheXF2NnRLM0ZrdnRFREdRVUhpY3ZK?=
 =?utf-8?B?UWFxZTU0bkNCbnlLaXVwZWdLZGM0NS9lNlY4cnFwamhNZWN2aENmMFcxVFRO?=
 =?utf-8?B?bktZTzgxcWI5QlMzSU84ellKK0J0blZPY0ZXM2ZGT1VZUGtlNFczSmhidy9M?=
 =?utf-8?Q?C1tUFb2ZO2v5O1dY9p1p7P2JQ?=
X-OriginatorOrg: labundy.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ab7b1354-e936-4b2d-7f96-08db5e33e8ec
X-MS-Exchange-CrossTenant-AuthSource: SN4PR0801MB3774.namprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 May 2023 21:55:25.0313
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 00b69d09-acab-4585-aca7-8fb7c6323e6f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: YATfJIJSH+9EF+6GuzDh+gQ4vxXt5h6g7s/M5sY1YwA47e5OFUmYY1gYlGjgzRycQWVuXsdqbJcs5CJ8ifo5eg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR08MB6700
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Fred,

On Fri, May 26, 2023 at 09:23:53PM +0000, Fred Treven wrote:
> 
> Hi Jeff,
> 
> > On May 26, 2023, at 2:43 PM, Jeff LaBundy <jeff@labundy.com> wrote:
> > 
> > Hi Fred,
> > 
> > On Thu, May 25, 2023 at 07:04:31PM -0500, Fred Treven wrote:
> >> Use MFD interface to load the CODEC driver along
> >> with the Input FF driver.
> >> 
> >> Signed-off-by: Fred Treven <fred.treven@cirrus.com>
> >> ---
> > 
> > Did you mean to include a thin codec driver as part of this series to
> > support the audio-to-haptics use-case? I don't see one.
> 
> That is the end-goal, but I wanted to submit a request for comment with just this patch to see if it was at all acceptable to add another device this way. I see now that it is not.
> 
> > 
> > As Lee correctly points out, this isn't an MFD despite the title of the
> > commit message, and is sort of an abuse of the API.
> 
> Understood. Do you think it’s best to migrate the appropriate code to the MFD subsystem before submitting this initial patchset (which will include the codec driver) or would it be acceptable to make that change after this has gone in? My hope was to avoid having code being reviewed more than once if a significant portion is moved to MFD.
> 
> > What you seem to actually want is a true MFD driver responsible for
> > initializing common resources such as regmap, an IRQ chip, etc. That
> > driver then adds input and codec drivers as children.
> > 
> > At the moment, you're more or less treating the input driver as the
> > MFD with one child, but that is not the correct pattern.
> 
> Yeah that makes sense. Please advise on what the best way to continue would be: a. Drop this patch and migrate to MFD after the Input driver has been accepted or b. Move necessary code to MFD and add both Input and codec drivers from there along with the codec driver.

If your goal is to support audio-to-haptics in mainline and to do so using an
ASoC driver, then I recommend option B. You should start with a scalable and
maintainable solution that reflects your long-term goals.

Option A is sunk cost. I also expect that you will identify additional changes
required to the input driver, particularly with respect to interrupt handling,
beyond moving initialization-related code to the MFD. It's a major re-write in
my opinion.

It would be interesting to see if an existing Cirrus ASoC driver can be gently
reworked to act as the child codec, vs. having to write an entirely new one for
L26. I'm sure the devil is in the details though.

> 
> Thank you,
> Fred
> 
> 

Kind regards,
Jeff LaBundy