All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Herve Codina <herve.codina@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Takashi Iwai <tiwai@suse.com>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v5 2/5] mfd: Add support for the Lantiq PEF2256 framer
Date: Thu, 30 Mar 2023 17:05:10 +0100	[thread overview]
Message-ID: <20230330160510.GB489249@google.com> (raw)
In-Reply-To: <20230328092645.634375-3-herve.codina@bootlin.com>

On Tue, 28 Mar 2023, Herve Codina wrote:

> The Lantiq PEF2256 is a framer and line interface component designed to
> fulfill all required interfacing between an analog E1/T1/J1 line and the
> digital PCM system highway/H.100 bus.

My goodness!

It's been a long time since I've seen anything quite like this.

My suggestion to you:

* Split this up into components that fit functional subsystems
* Run checkpatch.pl
* Remove all of the debug prints
* Move all of the defines out to a header file
* Be more verbose in your documentation / comments
* Consider using simple-mfd to probe child devices.

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/mfd/Kconfig         |   17 +
>  drivers/mfd/Makefile        |    1 +
>  drivers/mfd/pef2256.c       | 1355 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/pef2256.h |   28 +
>  4 files changed, 1401 insertions(+)
>  create mode 100644 drivers/mfd/pef2256.c
>  create mode 100644 include/linux/mfd/pef2256.h

--
Lee Jones [李琼斯]

WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee@kernel.org>
To: Herve Codina <herve.codina@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v5 2/5] mfd: Add support for the Lantiq PEF2256 framer
Date: Thu, 30 Mar 2023 17:05:10 +0100	[thread overview]
Message-ID: <20230330160510.GB489249@google.com> (raw)
In-Reply-To: <20230328092645.634375-3-herve.codina@bootlin.com>

On Tue, 28 Mar 2023, Herve Codina wrote:

> The Lantiq PEF2256 is a framer and line interface component designed to
> fulfill all required interfacing between an analog E1/T1/J1 line and the
> digital PCM system highway/H.100 bus.

My goodness!

It's been a long time since I've seen anything quite like this.

My suggestion to you:

* Split this up into components that fit functional subsystems
* Run checkpatch.pl
* Remove all of the debug prints
* Move all of the defines out to a header file
* Be more verbose in your documentation / comments
* Consider using simple-mfd to probe child devices.

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/mfd/Kconfig         |   17 +
>  drivers/mfd/Makefile        |    1 +
>  drivers/mfd/pef2256.c       | 1355 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/pef2256.h |   28 +
>  4 files changed, 1401 insertions(+)
>  create mode 100644 drivers/mfd/pef2256.c
>  create mode 100644 include/linux/mfd/pef2256.h

--
Lee Jones [李琼斯]

  reply	other threads:[~2023-03-30 16:06 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-28  9:26 [PATCH v5 0/5] Add the Lantiq PEF2256 audio support Herve Codina
2023-03-28  9:26 ` [PATCH v5 1/5] dt-bindings: mfd: Add the Lantiq PEF2256 E1/T1/J1 framer Herve Codina
2023-03-28 10:39   ` Krzysztof Kozlowski
2023-03-28  9:26 ` Herve Codina via Alsa-devel
2023-03-28  9:26 ` [PATCH v5 2/5] mfd: Add support for the Lantiq PEF2256 framer Herve Codina via Alsa-devel
2023-03-28  9:26 ` Herve Codina
2023-03-30 16:05   ` Lee Jones [this message]
2023-03-30 16:05     ` Lee Jones
2023-03-31  7:42     ` Herve Codina via Alsa-devel
2023-03-31  7:42     ` Herve Codina
2023-03-31  9:13       ` Krzysztof Kozlowski
2023-03-31  9:13         ` Krzysztof Kozlowski
2023-03-31 12:11         ` Herve Codina via Alsa-devel
2023-03-31 12:11         ` Herve Codina
2023-03-31 13:44           ` Krzysztof Kozlowski
2023-03-31 13:44             ` Krzysztof Kozlowski
2023-03-31 14:59             ` Herve Codina
2023-04-03 14:28               ` Lee Jones
2023-04-03 14:28                 ` Lee Jones
2023-04-04  7:20                 ` Herve Codina
2023-04-04  7:23                   ` Krzysztof Kozlowski
2023-04-04  7:23                     ` Krzysztof Kozlowski
2023-04-04  8:07                     ` Herve Codina via Alsa-devel
2023-04-04  8:07                     ` Herve Codina
2023-04-04  8:15                       ` Krzysztof Kozlowski
2023-04-04  8:15                         ` Krzysztof Kozlowski
2023-04-05 13:54                         ` Lee Jones
2023-04-05 13:54                           ` Lee Jones
2023-04-05 16:00                           ` Herve Codina via Alsa-devel
2023-04-05 16:00                           ` Herve Codina
2023-04-11 16:16                             ` Herve Codina
2023-04-12  9:59                               ` Lee Jones
2023-04-12  9:59                                 ` Lee Jones
2023-04-13  6:31                                 ` Herve Codina via Alsa-devel
2023-04-13  6:31                                 ` Herve Codina
2023-04-11 16:16                             ` Herve Codina via Alsa-devel
2023-04-04  7:20                 ` Herve Codina via Alsa-devel
2023-03-31 14:59             ` Herve Codina via Alsa-devel
2023-03-28  9:26 ` [PATCH v5 3/5] Documentation: sysfs: Document the Lantiq PEF2256 sysfs entry Herve Codina via Alsa-devel
2023-03-28  9:26 ` Herve Codina
2023-03-28  9:26 ` [PATCH v5 4/5] ASoC: codecs: Add support for the Lantiq PEF2256 codec Herve Codina via Alsa-devel
2023-03-28  9:26 ` Herve Codina
2023-03-28  9:26 ` [PATCH v5 5/5] MAINTAINERS: Add the Lantiq PEF2256 driver entry Herve Codina
2023-03-28  9:26 ` Herve Codina via Alsa-devel
  -- strict thread matches above, loose matches on Subject: below --
2023-04-03 23:56 [PATCH v5 2/5] mfd: Add support for the Lantiq PEF2256 framer kernel test robot

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=20230330160510.GB489249@google.com \
    --to=lee@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=devicetree@vger.kernel.org \
    --cc=herve.codina@bootlin.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    --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.