Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210604190338.2248295-1-martin.blumenstingl@googlemail.com>
 <20210604190338.2248295-3-martin.blumenstingl@googlemail.com> <YNAvItt8BQH1QmQT@vkoul-mobl>
In-Reply-To: <YNAvItt8BQH1QmQT@vkoul-mobl>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Tue, 22 Jun 2021 21:57:28 +0200
Message-ID: <CAFBinCAOi0EuucaQcJvgzV3o9fu=26u2ivAiQVidrmXTzQRQfg@mail.gmail.com>
Subject: Re: [PATCH 2/2] phy: amlogic: Add a new driver for the HDMI TX PHY on Meson8/8b/8m2
To:     Vinod Koul <vkoul@kernel.org>
Cc:     robh+dt@kernel.org, linux-phy@lists.infradead.org,
        devicetree@vger.kernel.org, linux-amlogic@lists.infradead.org,
        kishon@ti.com, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Vinod,

thanks for taking the time to review this!

On Mon, Jun 21, 2021 at 8:18 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 04-06-21, 21:03, Martin Blumenstingl wrote:
> > Amlogic Meson8/8b/8m2 have a built-in HDMI PHY in the HHI register
> > region. Unfortunately only few register bits are documented. For
> > HHI_HDMI_PHY_CNTL0 the magic numbers are taken from the 3.10 vendor
> > kernel.
> >
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > ---
> >  drivers/phy/amlogic/Kconfig              |  11 ++
> >  drivers/phy/amlogic/Makefile             |   1 +
> >  drivers/phy/amlogic/phy-meson8-hdmi-tx.c | 150 +++++++++++++++++++++++
> >  3 files changed, 162 insertions(+)
> >  create mode 100644 drivers/phy/amlogic/phy-meson8-hdmi-tx.c
> >
> > diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> > index db5d0cd757e3..e6c3a2a8b769 100644
> > --- a/drivers/phy/amlogic/Kconfig
> > +++ b/drivers/phy/amlogic/Kconfig
> > @@ -2,6 +2,17 @@
> >  #
> >  # Phy drivers for Amlogic platforms
> >  #
> > +config PHY_MESON8_HDMI_TX
> > +     tristate "Meson8, Meson8b and Meson8m2 HDMI TX PHY driver"
> > +     default ARCH_MESON
> > +     depends on (ARCH_MESON && ARM) || COMPILE_TEST
>
> depends on ARM seems redundant..
config ARCH_MESON exists in arch/arm as well as arch/arm64
my idea was to hide it from the 64-bit SoCs since it's not used there

While reading your comment I spotted something for myself:
I think "default ARCH_MESON" should not be there.

> > + * Meson8, Meson8b and Meson8m2 HDMI TX PHY.
> > + *
> > + * Copyright (C) 2020 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> 2021..
good catch, I'll fix this in v2


Best regards,
Martin