From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752009AbcENINY (ORCPT ); Sat, 14 May 2016 04:13:24 -0400 Received: from smtp04.smtpout.orange.fr ([80.12.242.126]:20556 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750975AbcENINM (ORCPT ); Sat, 14 May 2016 04:13:12 -0400 X-ME-Helo: belgarion X-ME-Auth: amFyem1pay5yb2JlcnRAb3JhbmdlLmZy X-ME-Date: Sat, 14 May 2016 10:13:09 +0200 X-ME-IP: 109.220.89.238 From: Robert Jarzmik To: Takashi Iwai Cc: "Haojian Zhuang" , "Liam Girdwood" , "Mark Brown" , "Jaroslav Kysela" , "Daniel Mack" , , , , Subject: Re: [RFC PATCH 0/7] AC97 device/driver model revamp References: <1462050939-27940-1-git-send-email-robert.jarzmik@free.fr> X-URL: http://belgarath.falguerolles.org/ Date: Sat, 14 May 2016 10:13:05 +0200 Message-ID: <87y47dhxbi.fsf@belgarion.home> User-Agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Takashi Iwai writes: > On Sat, 30 Apr 2016 23:15:32 +0200, > Robert Jarzmik wrote: >> >> Well, this is a long term effort, which might need a complete rewrite according >> to the comments it'll get. Let's expose it for comments and see how I can >> progress with it. > > I think it's good in general. The implementation looks fairly simple > and thin enough. Thanks. > An open question is whether migrating the former AC97 layer into the > new bus. I'm not sure about this. Transition to a new layer always > brings subtle bugs, especially when the target devices are in wide > range of legacy ones... If any, we should start just wrapping via > the new bus ops. I agree, the migration to the new layer will bring bugs. And the test might be impossible to carry out by lack of testers. The wrapping of bus operations is the goal of sound/ac97/snd_ac97_compat.c. > Some other nitpicks: > - We usually use snd_ prefix for sound stuff. Better to keep this for > exported symbols at least. Of course, for RFC v2. > - I don't see much value in the usefulness of compat_* stuff. That's mainly the bus operation wrapping and backward compatibility. The idea is to be able to convert only the probing/removal/suspend/resume of an ac97 codec or controller without changing its main code in a first patch, and then consider surgery (if applicable) to convert it wholly. > For example, it doesn't cover the actual reset procedure or such > done as in the old ac97 code. Ah then my code is incomplete and I must work on it. For the reset procedure I have ported snd_ac97_reset(), as well as the snd_ac97_us_ops operation wrappers. Would you give me an example of the reset I'm missing (a file and a function) ? > So it won't work compatibly. If it's a few lines of changes, the direct call > would be likely simpler in the end. > > - The order of patches needs reconsideration. The current patchset > will break the build, as the hook to sound/ac97/* is done in the > last patch, while you're already building against to the new stuff > beforehand. Very true. kbuild robot objected as well :) For RFC v2. Cheers. -- Robert From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Jarzmik Subject: Re: [RFC PATCH 0/7] AC97 device/driver model revamp Date: Sat, 14 May 2016 10:13:05 +0200 Message-ID: <87y47dhxbi.fsf@belgarion.home> References: <1462050939-27940-1-git-send-email-robert.jarzmik@free.fr> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.smtpout.orange.fr (smtp04.smtpout.orange.fr [80.12.242.126]) by alsa0.perex.cz (Postfix) with ESMTP id 3EF5E260542 for ; Sat, 14 May 2016 10:13:10 +0200 (CEST) List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, patches@opensource.wolfsonmicro.com, Liam Girdwood , Haojian Zhuang , Mark Brown , linux-arm-kernel@lists.infradead.org, Daniel Mack List-Id: alsa-devel@alsa-project.org Takashi Iwai writes: > On Sat, 30 Apr 2016 23:15:32 +0200, > Robert Jarzmik wrote: >> >> Well, this is a long term effort, which might need a complete rewrite according >> to the comments it'll get. Let's expose it for comments and see how I can >> progress with it. > > I think it's good in general. The implementation looks fairly simple > and thin enough. Thanks. > An open question is whether migrating the former AC97 layer into the > new bus. I'm not sure about this. Transition to a new layer always > brings subtle bugs, especially when the target devices are in wide > range of legacy ones... If any, we should start just wrapping via > the new bus ops. I agree, the migration to the new layer will bring bugs. And the test might be impossible to carry out by lack of testers. The wrapping of bus operations is the goal of sound/ac97/snd_ac97_compat.c. > Some other nitpicks: > - We usually use snd_ prefix for sound stuff. Better to keep this for > exported symbols at least. Of course, for RFC v2. > - I don't see much value in the usefulness of compat_* stuff. That's mainly the bus operation wrapping and backward compatibility. The idea is to be able to convert only the probing/removal/suspend/resume of an ac97 codec or controller without changing its main code in a first patch, and then consider surgery (if applicable) to convert it wholly. > For example, it doesn't cover the actual reset procedure or such > done as in the old ac97 code. Ah then my code is incomplete and I must work on it. For the reset procedure I have ported snd_ac97_reset(), as well as the snd_ac97_us_ops operation wrappers. Would you give me an example of the reset I'm missing (a file and a function) ? > So it won't work compatibly. If it's a few lines of changes, the direct call > would be likely simpler in the end. > > - The order of patches needs reconsideration. The current patchset > will break the build, as the hook to sound/ac97/* is done in the > last patch, while you're already building against to the new stuff > beforehand. Very true. kbuild robot objected as well :) For RFC v2. Cheers. -- Robert From mboxrd@z Thu Jan 1 00:00:00 1970 From: robert.jarzmik@free.fr (Robert Jarzmik) Date: Sat, 14 May 2016 10:13:05 +0200 Subject: [RFC PATCH 0/7] AC97 device/driver model revamp References: <1462050939-27940-1-git-send-email-robert.jarzmik@free.fr> Message-ID: <87y47dhxbi.fsf@belgarion.home> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Takashi Iwai writes: > On Sat, 30 Apr 2016 23:15:32 +0200, > Robert Jarzmik wrote: >> >> Well, this is a long term effort, which might need a complete rewrite according >> to the comments it'll get. Let's expose it for comments and see how I can >> progress with it. > > I think it's good in general. The implementation looks fairly simple > and thin enough. Thanks. > An open question is whether migrating the former AC97 layer into the > new bus. I'm not sure about this. Transition to a new layer always > brings subtle bugs, especially when the target devices are in wide > range of legacy ones... If any, we should start just wrapping via > the new bus ops. I agree, the migration to the new layer will bring bugs. And the test might be impossible to carry out by lack of testers. The wrapping of bus operations is the goal of sound/ac97/snd_ac97_compat.c. > Some other nitpicks: > - We usually use snd_ prefix for sound stuff. Better to keep this for > exported symbols at least. Of course, for RFC v2. > - I don't see much value in the usefulness of compat_* stuff. That's mainly the bus operation wrapping and backward compatibility. The idea is to be able to convert only the probing/removal/suspend/resume of an ac97 codec or controller without changing its main code in a first patch, and then consider surgery (if applicable) to convert it wholly. > For example, it doesn't cover the actual reset procedure or such > done as in the old ac97 code. Ah then my code is incomplete and I must work on it. For the reset procedure I have ported snd_ac97_reset(), as well as the snd_ac97_us_ops operation wrappers. Would you give me an example of the reset I'm missing (a file and a function) ? > So it won't work compatibly. If it's a few lines of changes, the direct call > would be likely simpler in the end. > > - The order of patches needs reconsideration. The current patchset > will break the build, as the hook to sound/ac97/* is done in the > last patch, while you're already building against to the new stuff > beforehand. Very true. kbuild robot objected as well :) For RFC v2. Cheers. -- Robert