* Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
From: Dmitry Baryshkov @ 2024-04-06 2:21 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-2-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:30AM +0200, Konrad Dybcio wrote:
> Introduce getters for SoC product and feature codes and export them.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/soc/qcom/smem.c | 66 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/soc/qcom/smem.h | 2 ++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 7191fa0c087f..e89b4d26877a 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -795,6 +795,72 @@ int qcom_smem_get_soc_id(u32 *id)
> }
> EXPORT_SYMBOL_GPL(qcom_smem_get_soc_id);
>
> +/**
> + * qcom_smem_get_feature_code() - return the feature code
> + * @id: On success, we return the feature code here.
> + *
> + * Look up the feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_feature_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->feature_code);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
> +
> +/**
> + * qcom_smem_get_product_code() - return the product code
> + * @id: On success, we return the product code here.
> + *
> + * Look up feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_product_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->pcode);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
This looks like a c&p from the previous function. Should we be comparing
the raw_code with a SOCINFO_PC_ constant?
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
> +
> static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
> {
> struct smem_header *header;
> diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
> index a36a3b9d4929..aef8c9fc6c08 100644
> --- a/include/linux/soc/qcom/smem.h
> +++ b/include/linux/soc/qcom/smem.h
> @@ -13,5 +13,7 @@ int qcom_smem_get_free_space(unsigned host);
> phys_addr_t qcom_smem_virt_to_phys(void *p);
>
> int qcom_smem_get_soc_id(u32 *id);
> +int qcom_smem_get_feature_code(u32 *code);
> +int qcom_smem_get_product_code(u32 *code);
>
> #endif
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 1/6] soc: qcom: Move some socinfo defines to the header, expand them
From: Dmitry Baryshkov @ 2024-04-06 2:22 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-1-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote:
> In preparation for parsing the chip "feature code" (FC) and "product
> code" (PC) (essentially the parameters that let us conclusively
> characterize the sillicon we're running on, including various speed
> bins), move the socinfo version defines to the public header and
> include some more FC/PC defines.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/soc/qcom/socinfo.c | 8 --------
> include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index 277c07a6603d..cf4616a468f2 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -21,14 +21,6 @@
>
> #include <dt-bindings/arm/qcom,ids.h>
>
> -/*
> - * SoC version type with major number in the upper 16 bits and minor
> - * number in the lower 16 bits.
> - */
> -#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
> -#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
> -#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff))
> -
> /* Helper macros to create soc_id table */
> #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
> #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
> diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h
> index e78777bb0f4a..ba7f683bd32c 100644
> --- a/include/linux/soc/qcom/socinfo.h
> +++ b/include/linux/soc/qcom/socinfo.h
> @@ -3,6 +3,8 @@
> #ifndef __QCOM_SOCINFO_H__
> #define __QCOM_SOCINFO_H__
>
> +#include <linux/types.h>
> +
> /*
> * SMEM item id, used to acquire handles to respective
> * SMEM region.
> @@ -12,6 +14,14 @@
> #define SMEM_SOCINFO_BUILD_ID_LENGTH 32
> #define SMEM_SOCINFO_CHIP_ID_LENGTH 32
>
> +/*
> + * SoC version type with major number in the upper 16 bits and minor
> + * number in the lower 16 bits.
> + */
> +#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
> +#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
> +#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff))
> +
> /* Socinfo SMEM item structure */
> struct socinfo {
> __le32 fmt;
> @@ -74,4 +84,30 @@ struct socinfo {
> __le32 boot_core;
> };
>
> +/* Internal feature codes */
> +enum feature_code {
> + /* External feature codes */
> + SOCINFO_FC_UNKNOWN = 0x0,
> + SOCINFO_FC_AA,
> + SOCINFO_FC_AB,
> + SOCINFO_FC_AC,
> + SOCINFO_FC_AD,
> + SOCINFO_FC_AE,
> + SOCINFO_FC_AF,
> + SOCINFO_FC_AG,
> + SOCINFO_FC_AH,
> + SOCINFO_FC_EXT_RESERVE,
> +};
> +
> +/* Internal feature codes */
> +/* Valid values: 0 <= n <= 0xf */
> +#define SOCINFO_FC_Yn(n) (0xf1 + n)
> +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10)
> +
> +/* Product codes */
> +#define SOCINFO_PC_UNKNOWN 0
> +/* Valid values: 0 <= n <= 8, the rest is reserved */
> +#define SOCINFO_PCn(n) (n + 1)
> +#define SOCINFO_PC_RESERVE (BIT(31) - 1)
Please move these defines into the next patch.
> +
> #endif
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2 06/18] PCI: endpoint: test: Implement link_down event operation
From: Manivannan Sadhasivam @ 2024-04-06 2:24 UTC (permalink / raw)
To: Niklas Cassel
Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <Zg//LiMg0Wh7LfT8@x1-carbon>
On Fri, Apr 05, 2024 at 03:39:58PM +0200, Niklas Cassel wrote:
> On Wed, Apr 03, 2024 at 01:18:23PM +0530, Manivannan Sadhasivam wrote:
> > On Sat, Mar 30, 2024 at 01:19:16PM +0900, Damien Le Moal wrote:
> > > Implement the link_down event operation to stop the command execution
> > > delayed work when the endpoint controller notifies a link down event.
> > >
> > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> >
> > This patch is already part of another series I posted [1] and under review. So
> > this can be dropped.
> >
> > - Mani
> >
> > [1] https://lore.kernel.org/linux-pci/20240401-pci-epf-rework-v2-9-970dbe90b99d@linaro.org/
>
> Mani, your patch does not use _sync(),
> so I don't think that we can simply drop this patch.
>
Agree, I was planning to update it in my next version anyway.
- Mani
>
> Kind regards,
> Niklas
>
> >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > drivers/pci/endpoint/functions/pci-epf-test.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index ab40c3182677..e6d4e1747c9f 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > @@ -824,9 +824,19 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
> > > return 0;
> > > }
> > >
> > > +static int pci_epf_test_link_down(struct pci_epf *epf)
> > > +{
> > > + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > > +
> > > + cancel_delayed_work_sync(&epf_test->cmd_handler);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> > > .core_init = pci_epf_test_core_init,
> > > .link_up = pci_epf_test_link_up,
> > > + .link_down = pci_epf_test_link_down,
> > > };
> > >
> > > static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > > --
> > > 2.44.0
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
From: Dmitry Baryshkov @ 2024-04-06 2:56 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-3-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> From: Neil Armstrong <neil.armstrong@linaro.org>
>
> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> the highest. Falling back to it when things go wrong is largely
> suboptimal, as more often than not, the top frequencies are not
> supposed to work on other bins.
Isn't it better to just return an error here instead of trying to guess
which speedbin to use?
If that's not the case, I think the commit should be expanded with
actually setting default_speedbin for the existing GPUs.
>
> Let the developer specify the intended "lowest common denominator" bin
> in struct adreno_info. If not specified, partial struct initialization
> will ensure it's set to zero, retaining previous behavior.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> [Konrad: clean up, add commit message]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 0674aca0f8a3..4cbdfabbcee5 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
> DRM_DEV_ERROR(dev,
> "missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
> speedbin);
> - supp_hw = BIT(0); /* Default */
> + supp_hw = BIT(info->default_speedbin); /* Default */
> }
>
> ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 77526892eb8c..460b399be37b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -110,6 +110,7 @@ struct adreno_info {
> * {SHRT_MAX, 0} sentinal.
> */
> struct adreno_speedbin *speedbins;
> + unsigned int default_speedbin;
> };
>
> #define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v6 00/16] power: sequencing: implement the subsystem and add first users
From: Xilin Wu @ 2024-04-06 3:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov
Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
Bartosz Golaszewski
In-Reply-To: <20240325131624.26023-1-brgl@bgdev.pl>
On 2024/3/25 21:16, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org>
>
> Note: I dropped most of the the review and test tags on purpose, the code
> changed significantly and warrants a new round of reviews and tests.
>
> ===
>
> Problem statement #1: Dynamic bus chicken-and-egg problem.
>
> Certain on-board PCI devices need to be powered up before they are can be
> detected but their PCI drivers won't get bound until the device is
> powered-up so enabling the relevant resources in the PCI device driver
> itself is impossible.
>
> Problem statement #2: Sharing inter-dependent resources between devices.
>
> Certain devices that use separate drivers (often on different busses)
> share resources (regulators, clocks, etc.). Typically these resources
> are reference-counted but in some cases there are additional interactions
> between them to consider, for example specific power-up sequence timings.
>
> ===
>
> The reason for tackling both of these problems in a single series is the
> fact the the platform I'm working on - Qualcomm RB5 - deals with both and
> both need to be addressed in order to enable WLAN and Bluetooth support
> upstream.
>
> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that
> takes inputs from the host and exposes LDO outputs consumed by the BT and
> WLAN modules which can be powered-up and down independently. However
> a delay of 100ms must be respected between enabling the BT- and
> WLAN-enable GPIOs.
>
> A similar design with a discreet PMU is also employed in other models of
> the WCN family of chips although we can often do without the delays. With
> this series we add support for the WCN7850 as well.
>
> ===
>
> We introduce a new subsystem here - the power sequencing framework. The
> qcom-wcn driver that we add is its first user. It implements the power-up
> sequences for QCA6390 and WCN7850 chips. However - we only use it to
> power-up the bluetooth module in the former. We use it to driver the WLAN
> modules in both. The reason for this is that for WCN7850 we have
> comprehensive bindings already upstream together with existing DT users.
> Porting them to using the pwrseq subsystem can be done separately and in
> an incremental manner once the subsystem itself is upstream. We will also
> have to ensure backward DT compatibility. To avoid overcomplicating this
> series, let's leave it out for now.
>
> ===
>
> This series is logically split into several sections. I'll go
> patch-by-patch and explain each step.
>
> Patches 1/16-5/16:
>
> These contain all relevant DT bindings changes. We add new documents for
> the QCA6390 & WCN7850 PMUs and ATH12K devices as well as extend the bindings
> for the Qualcomm Bluetooth and ATH11K modules with regulators used by them
> in QCA6390.
>
> Patches 6/16-8/16:
>
> These contain changes to device-tree sources for the three platforms we
> work with in this series. We model the PMUs of the WLAN/BT chips as
> top-level platform devices on the device tree. In order to limit the scope
> of this series and not introduce an excessive amount of confusion with
> deprecating DT bindings, we leave the Bluetooth nodes on sm8650 and sm8550
> as is (meaning: they continue to consumer the GPIOs and power inputs from
> the host). As the WCN7850 module doesn't require any specific timings, we can
> incrementally change that later.
>
> In both cases we add WLAN nodes that consume the power outputs of the PMU.
> For QCA6390 we also make the Bluetooth node of the RB5 consume the outputs
> of the PMU - we can do it as the bindings for this chip did not define any
> supply handles prior to this series meaning we are able to get this correct
> right away.
>
> Patches 9/16-12/16:
>
> These contain the bulk of the PCI changes for this series. We introduce
> a simple framework for powering up PCI devices before detecting them on
> the bus.
>
> The general approach is as follows: PCI devices that need special
> treatment before they can be powered up, scanned and bound to their PCI
> drivers must be described on the device-tree as child nodes of the PCI
> port node. These devices will be instantiated on the platform bus. They
> will in fact be generic platform devices with the compatible of the form
> used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We
> add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl
> drivers. These drivers are platform drivers that will now be matched
> against the devices instantiated from port children just like any other
> platform pairs.
>
> Both the power control platform device *AND* the associated PCI device
> reuse the same OF node and have access to the same properties. The goal
> of the platform driver is to request and bring up any required resources
> and let the pwrctl framework know that it's now OK to rescan the bus and
> detect the devices. When the device is bound, we are notified about it
> by the PCI bus notifier event and can establish a device link between the
> power control device and the PCI device so that any future extension for
> power-management will already be able to work with the correct hierachy.
>
> The reusing of the OF node is the reason for the small changes to the PCI
> OF core: as the bootloader can possibly leave the relevant regulators on
> before booting linux, the PCI device can be detected before its platform
> abstraction is probed. In this case, we find that device first and mark
> its OF node as reused. The pwrctl framework handles the opposite case
> (when the PCI device is detected only after the platform driver
> successfully enabled it).
>
> Patch 13/16 - 14/16:
>
> These add a relatively simple power sequencing subsystem and the first
> driver using it: the pwrseq module for the PMUs on the WCN family of chips.
>
> I'm proposing to add a subsystem that allows different devices to use a shared
> power sequence split into consumer-specific as well as common "units".
>
> A power sequence provider driver registers a set of units with pwrseq
> core. Each unit can be enabled and disabled and contains an optional list
> of other units which must be enabled before it itself can be. A unit
> represents a discreet chunk of the power sequence.
>
> It also registers a list of targets: a target is an abstraction wrapping
> a unit which allows consumers to tell pwrseq which unit they want to
> reach. Real-life example is the driver we're adding here: there's a set
> of common regulators, two PCIe-specific ones and two enable GPIOs: one
> for Bluetooth and one for WLAN.
>
> The Bluetooth driver requests a descriptor to the power sequencer and
> names the target it wants to reach:
>
> pwrseq = devm_pwrseq_get(dev, "bluetooth");
>
> The pwrseq core then knows that when the driver calls:
>
> pwrseq_power_on(pwrseq);
>
> It must enable the "bluetooth-enable" unit but it depends on the
> "regulators-common" unit so this one is enabled first. The provider
> driver is also in charge of assuring an appropriate delay between
> enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are
> handled by the "wlan-enable" unit and so are not enabled until the WLAN
> driver requests the "wlan" target to be powered on.
>
> Another thing worth discussing is the way we associate the consumer with
> the relevant power sequencer. DT maintainers have expressed a discontent
> with the existing mmc pwrseq bindings and have NAKed an earlier
> initiative to introduce global pwrseq bindings to the kernel[1].
>
> In this approach, we model the existing regulators and GPIOs in DT but
> the pwrseq subsystem requires each provider to provide a .match()
> callback. Whenever a consumer requests a power sequencer handle, we
> iterate over the list of pwrseq drivers and call .match() for each. It's
> up to the driver to verify in a platform-specific way whether it deals
> with its consumer and let the core pwrseq code know.
>
> The advantage of this over reusing the regulator or reset subsystem is
> that it's more generalized and can handle resources of all kinds as well
> as deal with any kind of power-on sequences: for instance, Qualcomm has
> a PCI switch they want a driver for but this switch requires enabling
> some resources first (PCI pwrctl) and then configuring the device over
> I2C (which can be handled by the pwrseq provider).
>
> Patch 15:
>
> This patch makes the Qualcomm Bluetooth driver get and use the power
> sequencer for QCA6390.
>
> Patch 16:
>
> While tiny, this patch is possibly the highlight of the entire series.
> It uses the two abstraction layers we introduced before to create an
> elegant power sequencing PCI power control driver and supports the ath11k
> module on QCA6390 and ath12k on WCN7850.
>
> With this series we can now enable BT and WLAN on several new Qualcomm
> boards upstream.
>
> Tested on RB5, sm8650-qrd and sm8550-qrd.
>
> Changelog:
>
> Since v5:
> - unify the approach to modelling the WCN WLAN/BT chips by always exposing
> the PMU node on the device tree and making the WLAN and BT nodes become
> consumers of its power outputs; this includes a major rework of the DT
> sources, bindings and driver code; there's no more a separate PCI
> pwrctl driver for WCN7850, instead its power-up sequence was moved
> into the pwrseq driver common for all WCN chips
> - don't set load_uA from new regulator consumers
> - fix reported kerneldoc issues
> - drop voltage ranges for PMU outputs from DT
> - many minor tweaks and reworks
>
> v1: Original RFC:
>
> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/
>
> v2: First real patch series (should have been PATCH v2) adding what I
> referred to back then as PCI power sequencing:
>
> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/
>
> v3: RFC for the DT representation of the PMU supplying the WLAN and BT
> modules inside the QCA6391 package (was largely separate from the
> series but probably should have been called PATCH or RFC v3):
>
> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/
>
> v4: Second attempt at the full series with changed scope (introduction of
> the pwrseq subsystem, should have been RFC v4)
>
> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/
>
> v5: Two different ways of handling QCA6390 and WCN7850:
>
> https://lore.kernel.org/lkml/20240216203215.40870-1-brgl@bgdev.pl/
>
> Bartosz Golaszewski (16):
> regulator: dt-bindings: describe the PMU module of the QCA6390 package
> regulator: dt-bindings: describe the PMU module of the WCN7850 package
> dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390
> dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on
> QCA6390
> dt-bindings: net: wireless: describe the ath12k PCI module
> arm64: dts: qcom: sm8550-qrd: add the Wifi node
> arm64: dts: qcom: sm8650-qrd: add the Wifi node
> arm64: dts: qcom: qrb5165-rb5: add the Wifi node
> PCI: hold the rescan mutex when scanning for the first time
> PCI/pwrctl: reuse the OF node for power controlled devices
> PCI/pwrctl: create platform devices for child OF nodes of the port
> node
> PCI/pwrctl: add PCI power control core code
> power: sequencing: implement the pwrseq core
> power: pwrseq: add a driver for the PMU module on the QCom WCN
> chipsets
> Bluetooth: qca: use the power sequencer for QCA6390
> PCI/pwrctl: add a PCI power control driver for power sequenced devices
>
> .../net/bluetooth/qualcomm-bluetooth.yaml | 17 +
> .../net/wireless/qcom,ath11k-pci.yaml | 46 +
> .../bindings/net/wireless/qcom,ath12k.yaml | 100 ++
> .../bindings/regulator/qcom,qca6390-pmu.yaml | 185 +++
> MAINTAINERS | 8 +
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 103 +-
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 +
> arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 ++
> arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 +
> arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 ++
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 +
> drivers/bluetooth/hci_qca.c | 74 +-
> drivers/pci/Kconfig | 1 +
> drivers/pci/Makefile | 1 +
> drivers/pci/bus.c | 9 +-
> drivers/pci/of.c | 14 +-
> drivers/pci/probe.c | 2 +
> drivers/pci/pwrctl/Kconfig | 17 +
> drivers/pci/pwrctl/Makefile | 6 +
> drivers/pci/pwrctl/core.c | 136 +++
> drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++
> drivers/pci/remove.c | 2 +
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/sequencing/Kconfig | 28 +
> drivers/power/sequencing/Makefile | 6 +
> drivers/power/sequencing/core.c | 1065 +++++++++++++++++
> drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 ++++++
> include/linux/pci-pwrctl.h | 51 +
> include/linux/pwrseq/consumer.h | 56 +
> include/linux/pwrseq/provider.h | 75 ++
> 31 files changed, 2614 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
> create mode 100644 drivers/pci/pwrctl/Kconfig
> create mode 100644 drivers/pci/pwrctl/Makefile
> create mode 100644 drivers/pci/pwrctl/core.c
> create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
> create mode 100644 drivers/power/sequencing/Kconfig
> create mode 100644 drivers/power/sequencing/Makefile
> create mode 100644 drivers/power/sequencing/core.c
> create mode 100644 drivers/power/sequencing/pwrseq-qcom-wcn.c
> create mode 100644 include/linux/pci-pwrctl.h
> create mode 100644 include/linux/pwrseq/consumer.h
> create mode 100644 include/linux/pwrseq/provider.h
I tested the patchset on SM8550 and it does give me working WiFi. However I
seethe following warnings during boot.
[ 5.973011] mhi mhi0: Requested to power ON
[ 6.597591] mhi mhi0: Power on setup success
[ 6.597631] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0'
[ 6.597634] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1
[ 6.597635] Hardware name: AYN Odin 2 (DT)
[ 6.597637] Workqueue: async async_run_entry_fn
[ 6.597645] Call trace:
[ 6.597646] dump_backtrace+0xa0/0x128
[ 6.597649] show_stack+0x20/0x38
[ 6.597650] dump_stack_lvl+0x74/0x90
[ 6.597653] dump_stack+0x18/0x28
[ 6.597654] sysfs_warn_dup+0x6c/0x90
[ 6.597658] sysfs_add_bin_file_mode_ns+0xdc/0x100
[ 6.597660] sysfs_create_bin_file+0x7c/0xb8
[ 6.597662] pci_create_attr+0xb4/0x1a8
[ 6.597665] pci_create_resource_files+0x64/0xd0
[ 6.597667] pci_create_sysfs_dev_files+0x24/0x40
[ 6.597669] pci_bus_add_device+0x54/0x138
[ 6.597670] pci_bus_add_devices+0x40/0x98
[ 6.597672] pci_host_probe+0x70/0xf0
[ 6.597673] dw_pcie_host_init+0x248/0x658
[ 6.597676] qcom_pcie_probe+0x234/0x330
[ 6.597677] platform_probe+0x70/0xd8
[ 6.597680] really_probe+0xc8/0x3a0
[ 6.597681] __driver_probe_device+0x84/0x170
[ 6.597682] driver_probe_device+0x44/0x120
[ 6.597683] __device_attach_driver+0xc4/0x168
[ 6.597684] bus_for_each_drv+0x8c/0xf0
[ 6.597686] __device_attach_async_helper+0xb4/0x118
[ 6.597687] async_run_entry_fn+0x40/0x178
[ 6.597689] process_one_work+0x16c/0x410
[ 6.597691] worker_thread+0x284/0x3a0
[ 6.597693] kthread+0x118/0x128
[ 6.597693] ret_from_fork+0x10/0x20
[ 6.597698] ------------[ cut here ]------------
[ 6.597698] proc_dir_entry '0000:00/00.0' already registered
[ 6.597710] WARNING: CPU: 7 PID: 154 at fs/proc/generic.c:375 proc_register+0x138/0x1d0
[ 6.597713] Modules linked in:
[ 6.597714] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1
[ 6.597715] Hardware name: AYN Odin 2 (DT)
[ 6.597716] Workqueue: async async_run_entry_fn
[ 6.597718] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 6.597719] pc : proc_register+0x138/0x1d0
[ 6.597721] lr : proc_register+0x138/0x1d0
[ 6.597723] sp : ffff800081e3b9a0
[ 6.597723] x29: ffff800081e3b9a0 x28: 0000000000000000 x27: ffffddb2a28eabe0
[ 6.597725] x26: ffff3425c9ada5c0 x25: ffffddb2a2d4eef0 x24: ffff3425c9ada540
[ 6.597726] x23: 0000000000000004 x22: ffff3425c7b1822c x21: 0000000000000004
[ 6.597727] x20: ffff3425c7b18180 x19: ffff3425c9adaec8 x18: ffffffffffffffff
[ 6.597729] x17: 3040636f732f6d72 x16: 6f6674616c702f73 x15: ffff800081e3b910
[ 6.597730] x14: 0000000000000000 x13: 0a64657265747369 x12: 6765722079646165
[ 6.597731] x11: fffffffffff00000 x10: ffffddb2a27c4fb0 x9 : ffffddb29f5d7528
[ 6.597733] x8 : 00000000ffff7fff x7 : ffffddb2a27c4fb0 x6 : 80000000ffff8000
[ 6.597734] x5 : 0000000000000358 x4 : 0000000000000000 x3 : 00000000ffffffff
[ 6.597736] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff3425c5ce0000
[ 6.597737] Call trace:
[ 6.597737] proc_register+0x138/0x1d0
[ 6.597739] proc_create_data+0x48/0x78
[ 6.597741] pci_proc_attach_device+0x84/0x118
[ 6.597743] pci_bus_add_device+0x5c/0x138
[ 6.597744] pci_bus_add_devices+0x40/0x98
[ 6.597745] pci_host_probe+0x70/0xf0
[ 6.597746] dw_pcie_host_init+0x248/0x658
[ 6.597748] qcom_pcie_probe+0x234/0x330
[ 6.597749] platform_probe+0x70/0xd8
[ 6.597750] really_probe+0xc8/0x3a0
[ 6.597751] __driver_probe_device+0x84/0x170
[ 6.597752] driver_probe_device+0x44/0x120
[ 6.597753] __device_attach_driver+0xc4/0x168
[ 6.597754] bus_for_each_drv+0x8c/0xf0
[ 6.597756] __device_attach_async_helper+0xb4/0x118
[ 6.597757] async_run_entry_fn+0x40/0x178
[ 6.597759] process_one_work+0x16c/0x410
[ 6.597760] worker_thread+0x284/0x3a0
[ 6.597761] kthread+0x118/0x128
[ 6.597762] ret_from_fork+0x10/0x20
[ 6.597763] ---[ end trace 0000000000000000 ]---
This probably only occurs when the relevant drivers on compiled as built-in.
Similar behavior has been noticed before as well:
https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/#mdeeca9bc8e19458787d53738298abcfff443068a
Thanks,
Xilin
^ permalink raw reply
* Re: [PATCH] arm64: dts: imx8mp: Align both CSI2 pixel clock
From: Marek Vasut @ 2024-04-06 2:58 UTC (permalink / raw)
To: Adam Ford, Laurent Pinchart
Cc: linux-arm-kernel, Conor Dooley, Fabio Estevam,
Krzysztof Kozlowski, Paul Elder, Pengutronix Kernel Team,
Rob Herring, Sascha Hauer, Shawn Guo, devicetree, imx
In-Reply-To: <CAHCN7xKX7v4tmhjvoPLirEoUG91jpu-8R2DV9eE=mnWt=3FffA@mail.gmail.com>
On 4/5/24 11:04 PM, Adam Ford wrote:
> On Fri, Apr 5, 2024 at 3:43 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Hi Marek,
>>
>> (CC'ing Adam)
>>
>> Thank you for the patch.
>>
>> On Fri, Apr 05, 2024 at 10:22:26PM +0200, Marek Vasut wrote:
>>> Configure both CSI2 assigned-clock-rates the same way.
>>> There does not seem to be any reason for keeping the
>>> two CSI2 pixel clock set to different frequencies.
>>
>> There's an issue when using two cameras concurrently. This has been
>> discussed some time ago on the linux-media mailing list, see [1]. Adam
>> knows more than I do on this topic.
>>
>> [1] https://lore.kernel.org/linux-media/CAHCN7x+kymRGO2kxvN2=zLiqRjfTc3hdf3VdNVkWjsW3La0bnA@mail.gmail.com/
>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Cc: Paul Elder <paul.elder@ideasonboard.com>
>>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: imx@lists.linux.dev
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> ---
>>> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> index 1bb96e96639f2..2e9ce0c3a9815 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> @@ -1703,7 +1703,7 @@ mipi_csi_1: csi@32e50000 {
>>> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
>>> assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
>>> <&clk IMX8MP_CLK_24M>;
>>> - assigned-clock-rates = <266000000>;
>>> + assigned-clock-rates = <500000000>;
>
> I am traveling, so I don't have the technical documents in front of
> me, but I beleive this is an over-drive speed, and 400MHz would be the
> single clock, standard rate. I created an imx8mm-overdrive and
> imx8mn-overdrive dtsi file to let users who operate in overdrive mode
> to update their clocks in one place.
>
> I also think this goes down if the user is running two cameras instead
> of one. I re-read the old thread, and it's coming back to me, but
> until I can get settled into my hotel in Germany, I won't have time to
> review. I think the original idea was to use the lowest, conservative
> value with the idea that people can tweak their clock settings if
> they're only running one and if they are running in over-drive mode.
MX8MPCEC does indeed read 400 MHz regular, 500 MHz overdrive.
Shall we align both CSI2 ports to 400 MHz ? Currently they are one 500
MHz and the other 266 MHz .
^ permalink raw reply
* Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
From: Dmitry Baryshkov @ 2024-04-06 3:23 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-4-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:32AM +0200, Konrad Dybcio wrote:
> On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is
> abstracted through SMEM, instead of being directly available in a fuse.
>
> Add support for SMEM-based speed binning, which includes getting
> "feature code" and "product code" from said source and parsing them
> to form something that lets us match OPPs against.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 +++---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 ++
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 39 +++++++++++++++++++++++++++---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 12 ++++++---
> 4 files changed, 51 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 4cbdfabbcee5..6776fd80f7a6 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2890,13 +2890,15 @@ static u32 fuse_to_supp_hw(const struct adreno_info *info, u32 fuse)
> return UINT_MAX;
> }
>
> -static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *info)
> +static int a6xx_set_supported_hw(struct adreno_gpu *adreno_gpu,
> + struct device *dev,
> + const struct adreno_info *info)
> {
> u32 supp_hw;
> u32 speedbin;
> int ret;
>
> - ret = adreno_read_speedbin(dev, &speedbin);
> + ret = adreno_read_speedbin(adreno_gpu, dev, &speedbin);
> /*
> * -ENOENT means that the platform doesn't support speedbin which is
> * fine
> @@ -3056,7 +3058,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> a6xx_llc_slices_init(pdev, a6xx_gpu, is_a7xx);
>
> - ret = a6xx_set_supported_hw(&pdev->dev, config->info);
> + ret = a6xx_set_supported_hw(adreno_gpu, &pdev->dev, config->info);
> if (ret) {
> a6xx_destroy(&(a6xx_gpu->base.base));
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index c3703a51287b..901ef767e491 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -6,6 +6,8 @@
> * Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
> */
>
> +#include <linux/soc/qcom/socinfo.h>
> +
> #include "adreno_gpu.h"
>
> bool hang_debug = false;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 074fb498706f..0e4ff532ac3c 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -21,6 +21,9 @@
> #include "msm_gem.h"
> #include "msm_mmu.h"
>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/soc/qcom/socinfo.h>
> +
> static u64 address_space_size = 0;
> MODULE_PARM_DESC(address_space_size, "Override for size of processes private GPU address space");
> module_param(address_space_size, ullong, 0600);
> @@ -1057,9 +1060,37 @@ void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem)
> adreno_ocmem->hdl);
> }
>
> -int adreno_read_speedbin(struct device *dev, u32 *speedbin)
> +int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
> + struct device *dev, u32 *speedbin)
> {
> - return nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
> + u32 fcode, pcode;
> + int ret;
> +
> + /* Try reading the speedbin via a nvmem cell first */
> + ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
> + if (!ret && ret != -EINVAL)
This is always false.
> + return ret;
> +
> + ret = qcom_smem_get_feature_code(&fcode);
> + if (ret) {
> + dev_err(dev, "Couldn't get feature code from SMEM!\n");
> + return ret;
This brings in QCOM_SMEM dependency (which is not mentioned in the
Kconfig). Please keep iMX5 hardware in mind, so the dependency should be
optional. Respective functions should be stubbed in the header.
> + }
> +
> + ret = qcom_smem_get_product_code(&pcode);
> + if (ret) {
> + dev_err(dev, "Couldn't get product code from SMEM!\n");
> + return ret;
> + }
> +
> + /* Don't consider fcode for external feature codes */
> + if (fcode <= SOCINFO_FC_EXT_RESERVE)
> + fcode = SOCINFO_FC_UNKNOWN;
> +
> + *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
> + FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
What about just asking the qcom_smem for the 'gpu_bin' and hiding gory
details there? It almost feels that handling raw PCODE / FCODE here is
too low-level and a subject to change depending on the socinfo format.
> +
> + return ret;
> }
>
> int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> @@ -1098,9 +1129,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> devm_pm_opp_set_clkname(dev, "core");
> }
>
> - if (adreno_read_speedbin(dev, &speedbin) || !speedbin)
> + if (adreno_read_speedbin(adreno_gpu, dev, &speedbin) || !speedbin)
> speedbin = 0xffff;
> - adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin);
the &= 0xffff should probably go to the adreno_read_speedbin / nvmem
case. WDYT?
> + adreno_gpu->speedbin = speedbin;
>
> gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%"ADRENO_CHIPID_FMT,
> ADRENO_CHIPID_ARGS(config->chip_id));
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 460b399be37b..1770a9e20484 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -81,7 +81,12 @@ extern const struct adreno_reglist a612_hwcg[], a615_hwcg[], a630_hwcg[], a640_h
> extern const struct adreno_reglist a660_hwcg[], a690_hwcg[], a702_hwcg[], a730_hwcg[], a740_hwcg[];
>
> struct adreno_speedbin {
> - uint16_t fuse;
> + /* <= 16-bit for NVMEM fuses, 32b for SOCID values */
> + uint32_t fuse;
> +#define ADRENO_SKU_ID_PCODE GENMASK(31, 16)
> +#define ADRENO_SKU_ID_FCODE GENMASK(15, 0)
> +#define ADRENO_SKU_ID(pcode, fcode) (pcode << 16 | fcode)
> +
> uint16_t speedbin;
> };
>
> @@ -137,7 +142,7 @@ struct adreno_gpu {
> struct msm_gpu base;
> const struct adreno_info *info;
> uint32_t chip_id;
> - uint16_t speedbin;
> + uint32_t speedbin;
> const struct adreno_gpu_funcs *funcs;
>
> /* interesting register offsets to dump: */
> @@ -520,7 +525,8 @@ int adreno_fault_handler(struct msm_gpu *gpu, unsigned long iova, int flags,
> struct adreno_smmu_fault_info *info, const char *block,
> u32 scratch[4]);
>
> -int adreno_read_speedbin(struct device *dev, u32 *speedbin);
> +int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
> + struct device *dev, u32 *speedbin);
>
> /*
> * For a5xx and a6xx targets load the zap shader that is used to pull the GPU
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 5/6] drm/msm/adreno: Add speedbin data for SM8550 / A740
From: Dmitry Baryshkov @ 2024-04-06 3:25 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-5-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:33AM +0200, Konrad Dybcio wrote:
> Add speebin data for A740, as found on SM8550 and derivative SoCs.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 901ef767e491..c976a485aef2 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -570,6 +570,20 @@ static const struct adreno_info gpulist[] = {
> .zapfw = "a740_zap.mdt",
> .hwcg = a740_hwcg,
> .address_space_size = SZ_16G,
> + .speedbins = ADRENO_SPEEDBINS(
I think this deserves either a comment or some info in the commit
message.
> + { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AC), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AF), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(1), SOCINFO_FC_UNKNOWN), 1 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x0)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x2)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x0)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x2)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x0)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x1)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xd)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xe)), 0 },
> + ),
> + .default_speedbin = 1,
> }, {
> .chip_ids = ADRENO_CHIP_IDS(0x43051401), /* "C520v2" */
> .family = ADRENO_7XX_GEN3,
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 0/6] Add SMEM-based speedbin matching
From: Dmitry Baryshkov @ 2024-04-06 3:28 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:28AM +0200, Konrad Dybcio wrote:
> Newer (SM8550+) SoCs don't seem to have a nice speedbin fuse anymore,
> but instead rely on a set of combinations of "feature code" (FC) and
> "product code" (PC) identifiers to match the bins. This series adds
> support for that.
>
> I suppose a qcom/for-soc immutable branch would be in order if we want
> to land this in the upcoming cycle.
>
> FWIW I preferred the fuses myself..
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Konrad Dybcio (5):
> soc: qcom: Move some socinfo defines to the header, expand them
> soc: qcom: smem: Add pcode/fcode getters
> drm/msm/adreno: Implement SMEM-based speed bin
> drm/msm/adreno: Add speedbin data for SM8550 / A740
> arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs
>
> Neil Armstrong (1):
> drm/msm/adreno: Allow specifying default speedbin value
Generic comment: as you are reworking speed bins implementaiton, could
you please take a broader look. A5xx just reads nvmem manually. A6xx
uses adreno_read_speedbin(). And then we call adreno_read_speedbin
second time from from adreno_gpu_init(). Can we get to the point where
the function is called only once for all the platforms which implements
speed binning?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 5/5] PCI: dwc: Add common send PME_Turn_Off message method
From: Manivannan Sadhasivam @ 2024-04-06 4:01 UTC (permalink / raw)
To: Frank Li
Cc: Bjorn Helgaas, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, imx, linux-pci, linux-kernel, devicetree
In-Reply-To: <ZhALNGyNTAzN86GF@lizhi-Precision-Tower-5810>
On Fri, Apr 05, 2024 at 10:31:16AM -0400, Frank Li wrote:
> On Fri, Apr 05, 2024 at 11:54:26AM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Mar 19, 2024 at 12:07:15PM -0400, Frank Li wrote:
> >
> > PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off during system suspend
> >
> > > Reserve space at end of first IORESOURCE_MEM window as message TLP MMIO
> > > window. This space's size is 'region_align'.
> > >
> > > Set outbound ATU map memory write to send PCI message. So one MMIO write
> > > can trigger a PCI message, such as PME_Turn_Off.
> > >
> > > Add common dwc_pme_turn_off() function.
> > >
> > > Call dwc_pme_turn_off() to send out PME_Turn_Off message in general
> > > dw_pcie_suspend_noirq() if there are not platform callback pme_turn_off()
> > > exist.
> > >
> >
> > How about:
> >
> > "Instead of relying on the vendor specific implementations to send the
> > PME_Turn_Off message, let's introduce a generic way of sending the message using
> > the MSG TLP.
> >
> > This is achieved by reserving a region for MSG TLP of size 'pci->region_align',
> > at the end of the first IORESOURCE_MEM window of the host bridge. And then
> > sending the PME_Turn_Off message during system suspend with the help of iATU.
> >
> > It should be noted that this generic implementation is optional for the glue
> > drivers and can be overridden by a custom 'pme_turn_off' callback."
> >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware-host.c | 94 ++++++++++++++++++++++-
> > > drivers/pci/controller/dwc/pcie-designware.h | 3 +
> > > 2 files changed, 93 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 267687ab33cbc..d5723fce7a894 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -393,6 +393,31 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> > > return 0;
> > > }
> > >
> > > +static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > + struct resource_entry *win;
> > > + struct resource *res;
> > > +
> > > + win = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> > > + if (win) {
> > > + res = devm_kzalloc(pci->dev, sizeof(*res), GFP_KERNEL);
> > > + if (!res)
> > > + return;
> > > +
> > > + /* Reserve last region_align block as message TLP space */
> > > + res->start = win->res->end - pci->region_align + 1;
> > > + res->end = win->res->end;
> >
> > Don't you need to adjust the host bridge window size and end address?
>
> Needn't. request_resource will reserve it from bridge window. Like malloc,
> if you malloc to get a region of memory, which will never get by malloc
> again utill you call free.
>
Hmm, will that modify the window->res->end address and size?
> >
> > > + res->name = "msg";
> > > + res->flags = win->res->flags | IORESOURCE_BUSY;
> > > +
> >
> > Shouldn't this resource be added back to the host bridge?
>
> No, this resource will reserver for msg only for whole bridge life cycle.
> Genenally alloc resource only happen at PCI devices probe. All pci space
> will be fixed after system probe.
>
I don't think so. This resource still belongs to the host bridge, so we should
add it back.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Manivannan Sadhasivam @ 2024-04-06 4:17 UTC (permalink / raw)
To: Mayank Rana
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
krzysztof.kozlowski+dt, conor+dt, devicetree, linux-arm-msm,
quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
In-Reply-To: <e2ff3031-bd71-4df7-a3a4-cec9c2339eaa@quicinc.com>
On Fri, Apr 05, 2024 at 10:41:15AM -0700, Mayank Rana wrote:
> Hi Mani
>
> On 4/4/2024 10:30 PM, Manivannan Sadhasivam wrote:
> > On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
> > > On some of Qualcomm platform, firmware configures PCIe controller into
> > > ECAM mode allowing static memory allocation for configuration space of
> > > supported bus range. Firmware also takes care of bringing up PCIe PHY
> > > and performing required operation to bring PCIe link into D0. Firmware
> > > also manages system resources (e.g. clocks/regulators/resets/ bus voting).
> > > Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
> > > root complex and connected PCIe devices. Firmware won't be enumerating
> > > or powering up PCIe root complex until this driver invokes power domain
> > > based notification to bring PCIe link into D0/D3cold mode.
> > >
> >
> > Is this an in-house PCIe IP of Qualcomm or the same DWC IP that is used in other
> > SoCs?
> >
> > - Mani
> Driver is validated on SA8775p-ride platform using PCIe DWC IP for
> now.Although this driver doesn't need to know used PCIe controller and PHY
> IP as well programming sequence as that would be taken care by firmware.
>
Ok, so it is the same IP but firmware is controlling the resources now. This
information should be present in the commit message.
Btw, there is an existing generic ECAM host controller driver:
drivers/pci/controller/pci-host-generic.c
This driver is already being used by several vendors as well. So we should try
to extend it for Qcom usecase also.
> > > This driver also support MSI functionality using PCIe controller based
> > > MSI controller as GIC ITS based MSI functionality is not available on
> > > some of platform.
> > >
So is this the same internal MSI controller in the DWC IP? If so, then we
already have the MSI implementation in
drivers/pci/controller/dwc/pcie-designware-host.c and that should be reused here
instead of duplicating the code.
- Mani
> > > Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> > > ---
> > > drivers/pci/controller/Kconfig | 12 +
> > > drivers/pci/controller/Makefile | 1 +
> > > drivers/pci/controller/pcie-qcom-ecam.c | 575 ++++++++++++++++++++++++++++++++
> > > 3 files changed, 588 insertions(+)
> > > create mode 100644 drivers/pci/controller/pcie-qcom-ecam.c
> > >
> > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> > > index e534c02..abbd9f2 100644
> > > --- a/drivers/pci/controller/Kconfig
> > > +++ b/drivers/pci/controller/Kconfig
> > > @@ -353,6 +353,18 @@ config PCIE_XILINX_CPM
> > > Say 'Y' here if you want kernel support for the
> > > Xilinx Versal CPM host bridge.
> > > +config PCIE_QCOM_ECAM
> > > + tristate "QCOM PCIe ECAM host controller"
> > > + depends on ARCH_QCOM && PCI
> > > + depends on OF
> > > + select PCI_MSI
> > > + select PCI_HOST_COMMON
> > > + select IRQ_DOMAIN
> > > + help
> > > + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
> > > + PCIe root host controller. The controller is programmed using firmware
> > > + to support ECAM compatible memory address space.
> > > +
> > > source "drivers/pci/controller/cadence/Kconfig"
> > > source "drivers/pci/controller/dwc/Kconfig"
> > > source "drivers/pci/controller/mobiveil/Kconfig"
> > > diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> > > index f2b19e6..2f1ee1e 100644
> > > --- a/drivers/pci/controller/Makefile
> > > +++ b/drivers/pci/controller/Makefile
> > > @@ -40,6 +40,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> > > obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> > > obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
> > > obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
> > > +obj-$(CONFIG_PCIE_QCOM_ECAM) += pcie-qcom-ecam.o
> > > # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> > > obj-y += dwc/
> > > diff --git a/drivers/pci/controller/pcie-qcom-ecam.c b/drivers/pci/controller/pcie-qcom-ecam.c
> > > new file mode 100644
> > > index 00000000..5b4c68b
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/pcie-qcom-ecam.c
> > > @@ -0,0 +1,575 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Qualcomm PCIe ECAM root host controller driver
> > > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> > > + */
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include <linux/irq.h>
> > > +#include <linux/irqchip/chained_irq.h>
> > > +#include <linux/irqdomain.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/msi.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/of_irq.h>
> > > +#include <linux/of_pci.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/pci-ecam.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_domain.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/types.h>
> > > +
> > > +#define PCIE_MSI_CTRL_BASE (0x820)
> > > +#define PCIE_MSI_CTRL_SIZE (0x68)
> > > +#define PCIE_MSI_CTRL_ADDR_OFFS (0x0)
> > > +#define PCIE_MSI_CTRL_UPPER_ADDR_OFFS (0x4)
> > > +#define PCIE_MSI_CTRL_INT_N_EN_OFFS(n) (0x8 + 0xc * (n))
> > > +#define PCIE_MSI_CTRL_INT_N_MASK_OFFS(n) (0xc + 0xc * (n))
> > > +#define PCIE_MSI_CTRL_INT_N_STATUS_OFFS(n) (0x10 + 0xc * (n))
> > > +
> > > +#define MSI_DB_ADDR 0xa0000000
> > > +#define MSI_IRQ_PER_GRP (32)
> > > +
> > > +/**
> > > + * struct qcom_msi_irq - MSI IRQ information
> > > + * @client: pointer to MSI client struct
> > > + * @grp: group the irq belongs to
> > > + * @grp_index: index in group
> > > + * @hwirq: hwirq number
> > > + * @virq: virq number
> > > + * @pos: position in MSI bitmap
> > > + */
> > > +struct qcom_msi_irq {
> > > + struct qcom_msi_client *client;
> > > + struct qcom_msi_grp *grp;
> > > + unsigned int grp_index;
> > > + unsigned int hwirq;
> > > + unsigned int virq;
> > > + u32 pos;
> > > +};
> > > +
> > > +/**
> > > + * struct qcom_msi_grp - MSI group information
> > > + * @int_en_reg: memory-mapped interrupt enable register address
> > > + * @int_mask_reg: memory-mapped interrupt mask register address
> > > + * @int_status_reg: memory-mapped interrupt status register address
> > > + * @mask: tracks masked/unmasked MSI
> > > + * @irqs: structure to MSI IRQ information
> > > + */
> > > +struct qcom_msi_grp {
> > > + void __iomem *int_en_reg;
> > > + void __iomem *int_mask_reg;
> > > + void __iomem *int_status_reg;
> > > + u32 mask;
> > > + struct qcom_msi_irq irqs[MSI_IRQ_PER_GRP];
> > > +};
> > > +
> > > +/**
> > > + * struct qcom_msi - PCIe controller based MSI controller information
> > > + * @clients: list for tracking clients
> > > + * @dev: platform device node
> > > + * @nr_hwirqs: total number of hardware IRQs
> > > + * @nr_virqs: total number of virqs
> > > + * @nr_grps: total number of groups
> > > + * @grps: pointer to all groups information
> > > + * @bitmap: tracks used/unused MSI
> > > + * @mutex: for modifying MSI client list and bitmap
> > > + * @inner_domain: parent domain; gen irq related
> > > + * @msi_domain: child domain; pcie related
> > > + * @msi_db_addr: MSI doorbell address
> > > + * @cfg_lock: lock for configuring MSI controller registers
> > > + * @pcie_msi_cfg: memory-mapped MSI controller register space
> > > + */
> > > +struct qcom_msi {
> > > + struct list_head clients;
> > > + struct device *dev;
> > > + int nr_hwirqs;
> > > + int nr_virqs;
> > > + int nr_grps;
> > > + struct qcom_msi_grp *grps;
> > > + unsigned long *bitmap;
> > > + struct mutex mutex;
> > > + struct irq_domain *inner_domain;
> > > + struct irq_domain *msi_domain;
> > > + phys_addr_t msi_db_addr;
> > > + spinlock_t cfg_lock;
> > > + void __iomem *pcie_msi_cfg;
> > > +};
> > > +
> > > +/**
> > > + * struct qcom_msi_client - structure for each client of MSI controller
> > > + * @node: list to track number of MSI clients
> > > + * @msi: client specific MSI controller based resource pointer
> > > + * @dev: client's dev of pci_dev
> > > + * @nr_irqs: number of irqs allocated for client
> > > + * @msi_addr: MSI doorbell address
> > > + */
> > > +struct qcom_msi_client {
> > > + struct list_head node;
> > > + struct qcom_msi *msi;
> > > + struct device *dev;
> > > + unsigned int nr_irqs;
> > > + phys_addr_t msi_addr;
> > > +};
> > > +
> > > +static void qcom_msi_handler(struct irq_desc *desc)
> > > +{
> > > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > > + struct qcom_msi_grp *msi_grp;
> > > + u32 status;
> > > + int i;
> > > +
> > > + chained_irq_enter(chip, desc);
> > > +
> > > + msi_grp = irq_desc_get_handler_data(desc);
> > > + status = readl_relaxed(msi_grp->int_status_reg);
> > > + status ^= (msi_grp->mask & status);
> > > + writel(status, msi_grp->int_status_reg);
> > > +
> > > + for (i = 0; status; i++, status >>= 1)
> > > + if (status & 0x1)
> > > + generic_handle_irq(msi_grp->irqs[i].virq);
> > > +
> > > + chained_irq_exit(chip, desc);
> > > +}
> > > +
> > > +static void qcom_msi_mask_irq(struct irq_data *data)
> > > +{
> > > + struct irq_data *parent_data;
> > > + struct qcom_msi_irq *msi_irq;
> > > + struct qcom_msi_grp *msi_grp;
> > > + struct qcom_msi *msi;
> > > + unsigned long flags;
> > > +
> > > + parent_data = data->parent_data;
> > > + if (!parent_data)
> > > + return;
> > > +
> > > + msi_irq = irq_data_get_irq_chip_data(parent_data);
> > > + msi = msi_irq->client->msi;
> > > + msi_grp = msi_irq->grp;
> > > +
> > > + spin_lock_irqsave(&msi->cfg_lock, flags);
> > > + pci_msi_mask_irq(data);
> > > + msi_grp->mask |= BIT(msi_irq->grp_index);
> > > + writel(msi_grp->mask, msi_grp->int_mask_reg);
> > > + spin_unlock_irqrestore(&msi->cfg_lock, flags);
> > > +}
> > > +
> > > +static void qcom_msi_unmask_irq(struct irq_data *data)
> > > +{
> > > + struct irq_data *parent_data;
> > > + struct qcom_msi_irq *msi_irq;
> > > + struct qcom_msi_grp *msi_grp;
> > > + struct qcom_msi *msi;
> > > + unsigned long flags;
> > > +
> > > + parent_data = data->parent_data;
> > > + if (!parent_data)
> > > + return;
> > > +
> > > + msi_irq = irq_data_get_irq_chip_data(parent_data);
> > > + msi = msi_irq->client->msi;
> > > + msi_grp = msi_irq->grp;
> > > +
> > > + spin_lock_irqsave(&msi->cfg_lock, flags);
> > > + msi_grp->mask &= ~BIT(msi_irq->grp_index);
> > > + writel(msi_grp->mask, msi_grp->int_mask_reg);
> > > + pci_msi_unmask_irq(data);
> > > + spin_unlock_irqrestore(&msi->cfg_lock, flags);
> > > +}
> > > +
> > > +static struct irq_chip qcom_msi_irq_chip = {
> > > + .name = "qcom_pci_msi",
> > > + .irq_enable = qcom_msi_unmask_irq,
> > > + .irq_disable = qcom_msi_mask_irq,
> > > + .irq_mask = qcom_msi_mask_irq,
> > > + .irq_unmask = qcom_msi_unmask_irq,
> > > +};
> > > +
> > > +static int qcom_msi_domain_prepare(struct irq_domain *domain, struct device *dev,
> > > + int nvec, msi_alloc_info_t *arg)
> > > +{
> > > + struct qcom_msi *msi = domain->parent->host_data;
> > > + struct qcom_msi_client *client;
> > > +
> > > + client = kzalloc(sizeof(*client), GFP_KERNEL);
> > > + if (!client)
> > > + return -ENOMEM;
> > > +
> > > + client->msi = msi;
> > > + client->dev = dev;
> > > + client->msi_addr = msi->msi_db_addr;
> > > + mutex_lock(&msi->mutex);
> > > + list_add_tail(&client->node, &msi->clients);
> > > + mutex_unlock(&msi->mutex);
> > > +
> > > + /* zero out struct for pcie msi framework */
> > > + memset(arg, 0, sizeof(*arg));
> > > + return 0;
> > > +}
> > > +
> > > +static struct msi_domain_ops qcom_msi_domain_ops = {
> > > + .msi_prepare = qcom_msi_domain_prepare,
> > > +};
> > > +
> > > +static struct msi_domain_info qcom_msi_domain_info = {
> > > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> > > + MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
> > > + .ops = &qcom_msi_domain_ops,
> > > + .chip = &qcom_msi_irq_chip,
> > > +};
> > > +
> > > +static int qcom_msi_irq_set_affinity(struct irq_data *data,
> > > + const struct cpumask *mask, bool force)
> > > +{
> > > + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> > > + int ret = 0;
> > > +
> > > + if (!parent_data)
> > > + return -ENODEV;
> > > +
> > > + /* set affinity for MSI HW IRQ */
> > > + if (parent_data->chip->irq_set_affinity)
> > > + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> > > +{
> > > + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> > > + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
> > > + struct qcom_msi_client *client = msi_irq->client;
> > > +
> > > + if (!parent_data)
> > > + return;
> > > +
> > > + msg->address_lo = lower_32_bits(client->msi_addr);
> > > + msg->address_hi = upper_32_bits(client->msi_addr);
> > > + msg->data = msi_irq->pos;
> > > +}
> > > +
> > > +static struct irq_chip qcom_msi_bottom_irq_chip = {
> > > + .name = "qcom_msi",
> > > + .irq_set_affinity = qcom_msi_irq_set_affinity,
> > > + .irq_compose_msi_msg = qcom_msi_irq_compose_msi_msg,
> > > +};
> > > +
> > > +static int qcom_msi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> > > + unsigned int nr_irqs, void *args)
> > > +{
> > > + struct device *dev = ((msi_alloc_info_t *)args)->desc->dev;
> > > + struct qcom_msi_client *tmp, *client = NULL;
> > > + struct qcom_msi *msi = domain->host_data;
> > > + int i, ret = 0;
> > > + int pos;
> > > +
> > > + mutex_lock(&msi->mutex);
> > > + list_for_each_entry(tmp, &msi->clients, node) {
> > > + if (tmp->dev == dev) {
> > > + client = tmp;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (!client) {
> > > + dev_err(msi->dev, "failed to find MSI client dev\n");
> > > + ret = -ENODEV;
> > > + goto out;
> > > + }
> > > +
> > > + pos = bitmap_find_next_zero_area(msi->bitmap, msi->nr_virqs, 0,
> > > + nr_irqs, nr_irqs - 1);
> > > + if (pos > msi->nr_virqs) {
> > > + ret = -ENOSPC;
> > > + goto out;
> > > + }
> > > +
> > > + bitmap_set(msi->bitmap, pos, nr_irqs);
> > > + for (i = 0; i < nr_irqs; i++) {
> > > + u32 grp = pos / MSI_IRQ_PER_GRP;
> > > + u32 index = pos % MSI_IRQ_PER_GRP;
> > > + struct qcom_msi_irq *msi_irq = &msi->grps[grp].irqs[index];
> > > +
> > > + msi_irq->virq = virq + i;
> > > + msi_irq->client = client;
> > > + irq_domain_set_info(domain, msi_irq->virq,
> > > + msi_irq->hwirq,
> > > + &qcom_msi_bottom_irq_chip, msi_irq,
> > > + handle_simple_irq, NULL, NULL);
> > > + client->nr_irqs++;
> > > + pos++;
> > > + }
> > > +out:
> > > + mutex_unlock(&msi->mutex);
> > > + return ret;
> > > +}
> > > +
> > > +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> > > + unsigned int nr_irqs)
> > > +{
> > > + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> > > + struct qcom_msi_client *client;
> > > + struct qcom_msi_irq *msi_irq;
> > > + struct qcom_msi *msi;
> > > +
> > > + if (!data)
> > > + return;
> > > +
> > > + msi_irq = irq_data_get_irq_chip_data(data);
> > > + client = msi_irq->client;
> > > + msi = client->msi;
> > > +
> > > + mutex_lock(&msi->mutex);
> > > + bitmap_clear(msi->bitmap, msi_irq->pos, nr_irqs);
> > > +
> > > + client->nr_irqs -= nr_irqs;
> > > + if (!client->nr_irqs) {
> > > + list_del(&client->node);
> > > + kfree(client);
> > > + }
> > > + mutex_unlock(&msi->mutex);
> > > +
> > > + irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> > > +}
> > > +
> > > +static const struct irq_domain_ops msi_domain_ops = {
> > > + .alloc = qcom_msi_irq_domain_alloc,
> > > + .free = qcom_msi_irq_domain_free,
> > > +};
> > > +
> > > +static int qcom_msi_alloc_domains(struct qcom_msi *msi)
> > > +{
> > > + msi->inner_domain = irq_domain_add_linear(NULL, msi->nr_virqs,
> > > + &msi_domain_ops, msi);
> > > + if (!msi->inner_domain) {
> > > + dev_err(msi->dev, "failed to create IRQ inner domain\n");
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->dev->of_node),
> > > + &qcom_msi_domain_info, msi->inner_domain);
> > > + if (!msi->msi_domain) {
> > > + dev_err(msi->dev, "failed to create MSI domain\n");
> > > + irq_domain_remove(msi->inner_domain);
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int qcom_msi_irq_setup(struct qcom_msi *msi)
> > > +{
> > > + struct qcom_msi_grp *msi_grp;
> > > + struct qcom_msi_irq *msi_irq;
> > > + int i, index, ret;
> > > + unsigned int irq;
> > > +
> > > + /* setup each MSI group. nr_hwirqs == nr_grps */
> > > + for (i = 0; i < msi->nr_hwirqs; i++) {
> > > + irq = irq_of_parse_and_map(msi->dev->of_node, i);
> > > + if (!irq) {
> > > + dev_err(msi->dev,
> > > + "MSI: failed to parse/map interrupt\n");
> > > + ret = -ENODEV;
> > > + goto free_irqs;
> > > + }
> > > +
> > > + msi_grp = &msi->grps[i];
> > > + msi_grp->int_en_reg = msi->pcie_msi_cfg +
> > > + PCIE_MSI_CTRL_INT_N_EN_OFFS(i);
> > > + msi_grp->int_mask_reg = msi->pcie_msi_cfg +
> > > + PCIE_MSI_CTRL_INT_N_MASK_OFFS(i);
> > > + msi_grp->int_status_reg = msi->pcie_msi_cfg +
> > > + PCIE_MSI_CTRL_INT_N_STATUS_OFFS(i);
> > > +
> > > + for (index = 0; index < MSI_IRQ_PER_GRP; index++) {
> > > + msi_irq = &msi_grp->irqs[index];
> > > +
> > > + msi_irq->grp = msi_grp;
> > > + msi_irq->grp_index = index;
> > > + msi_irq->pos = (i * MSI_IRQ_PER_GRP) + index;
> > > + msi_irq->hwirq = irq;
> > > + }
> > > +
> > > + irq_set_chained_handler_and_data(irq, qcom_msi_handler, msi_grp);
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +free_irqs:
> > > + for (--i; i >= 0; i--) {
> > > + irq = msi->grps[i].irqs[0].hwirq;
> > > +
> > > + irq_set_chained_handler_and_data(irq, NULL, NULL);
> > > + irq_dispose_mapping(irq);
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void qcom_msi_config(struct irq_domain *domain)
> > > +{
> > > + struct qcom_msi *msi;
> > > + int i;
> > > +
> > > + msi = domain->parent->host_data;
> > > +
> > > + /* program termination address */
> > > + writel(msi->msi_db_addr, msi->pcie_msi_cfg + PCIE_MSI_CTRL_ADDR_OFFS);
> > > + writel(0, msi->pcie_msi_cfg + PCIE_MSI_CTRL_UPPER_ADDR_OFFS);
> > > +
> > > + /* restore mask and enable all interrupts for each group */
> > > + for (i = 0; i < msi->nr_grps; i++) {
> > > + struct qcom_msi_grp *msi_grp = &msi->grps[i];
> > > +
> > > + writel(msi_grp->mask, msi_grp->int_mask_reg);
> > > + writel(~0, msi_grp->int_en_reg);
> > > + }
> > > +}
> > > +
> > > +static void qcom_msi_deinit(struct qcom_msi *msi)
> > > +{
> > > + irq_domain_remove(msi->msi_domain);
> > > + irq_domain_remove(msi->inner_domain);
> > > +}
> > > +
> > > +static struct qcom_msi *qcom_msi_init(struct device *dev)
> > > +{
> > > + struct qcom_msi *msi;
> > > + u64 addr;
> > > + int ret;
> > > +
> > > + msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
> > > + if (!msi)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + msi->dev = dev;
> > > + mutex_init(&msi->mutex);
> > > + spin_lock_init(&msi->cfg_lock);
> > > + INIT_LIST_HEAD(&msi->clients);
> > > +
> > > + msi->msi_db_addr = MSI_DB_ADDR;
> > > + msi->nr_hwirqs = of_irq_count(dev->of_node);
> > > + if (!msi->nr_hwirqs) {
> > > + dev_err(msi->dev, "no hwirqs found\n");
> > > + return ERR_PTR(-ENODEV);
> > > + }
> > > +
> > > + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
> > > + dev_err(msi->dev, "failed to get reg address\n");
> > > + return ERR_PTR(-ENODEV);
> > > + }
> > > +
> > > + dev_dbg(msi->dev, "hwirq:%d pcie_msi_cfg:%llx\n", msi->nr_hwirqs, addr);
> > > + msi->pcie_msi_cfg = devm_ioremap(dev, addr + PCIE_MSI_CTRL_BASE, PCIE_MSI_CTRL_SIZE);
> > > + if (!msi->pcie_msi_cfg)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + msi->nr_virqs = msi->nr_hwirqs * MSI_IRQ_PER_GRP;
> > > + msi->nr_grps = msi->nr_hwirqs;
> > > + msi->grps = devm_kcalloc(dev, msi->nr_grps, sizeof(*msi->grps), GFP_KERNEL);
> > > + if (!msi->grps)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + msi->bitmap = devm_kcalloc(dev, BITS_TO_LONGS(msi->nr_virqs),
> > > + sizeof(*msi->bitmap), GFP_KERNEL);
> > > + if (!msi->bitmap)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + ret = qcom_msi_alloc_domains(msi);
> > > + if (ret)
> > > + return ERR_PTR(ret);
> > > +
> > > + ret = qcom_msi_irq_setup(msi);
> > > + if (ret) {
> > > + qcom_msi_deinit(msi);
> > > + return ERR_PTR(ret);
> > > + }
> > > +
> > > + qcom_msi_config(msi->msi_domain);
> > > + return msi;
> > > +}
> > > +
> > > +static int qcom_pcie_ecam_suspend_noirq(struct device *dev)
> > > +{
> > > + return pm_runtime_put_sync(dev);
> > > +}
> > > +
> > > +static int qcom_pcie_ecam_resume_noirq(struct device *dev)
> > > +{
> > > + return pm_runtime_get_sync(dev);
> > > +}
> > > +
> > > +static int qcom_pcie_ecam_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct qcom_msi *msi;
> > > + int ret;
> > > +
> > > + ret = devm_pm_runtime_enable(dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = pm_runtime_resume_and_get(dev);
> > > + if (ret < 0) {
> > > + dev_err(dev, "fail to enable pcie controller: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + msi = qcom_msi_init(dev);
> > > + if (IS_ERR(msi)) {
> > > + pm_runtime_put_sync(dev);
> > > + return PTR_ERR(msi);
> > > + }
> > > +
> > > + ret = pci_host_common_probe(pdev);
> > > + if (ret) {
> > > + dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
> > > + qcom_msi_deinit(msi);
> > > + pm_runtime_put_sync(dev);
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static const struct dev_pm_ops qcom_pcie_ecam_pm_ops = {
> > > + NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_ecam_suspend_noirq,
> > > + qcom_pcie_ecam_resume_noirq)
> > > +};
> > > +
> > > +static const struct pci_ecam_ops qcom_pcie_ecam_ops = {
> > > + .pci_ops = {
> > > + .map_bus = pci_ecam_map_bus,
> > > + .read = pci_generic_config_read,
> > > + .write = pci_generic_config_write,
> > > + }
> > > +};
> > > +
> > > +static const struct of_device_id qcom_pcie_ecam_of_match[] = {
> > > + {
> > > + .compatible = "qcom,pcie-ecam-rc",
> > > + .data = &qcom_pcie_ecam_ops,
> > > + },
> > > + { },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, qcom_pcie_ecam_of_match);
> > > +
> > > +static struct platform_driver qcom_pcie_ecam_driver = {
> > > + .probe = qcom_pcie_ecam_probe,
> > > + .driver = {
> > > + .name = "qcom-pcie-ecam-rc",
> > > + .suppress_bind_attrs = true,
> > > + .of_match_table = qcom_pcie_ecam_of_match,
> > > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > > + .pm = &qcom_pcie_ecam_pm_ops,
> > > + },
> > > +};
> > > +module_platform_driver(qcom_pcie_ecam_driver);
> > > +
> > > +MODULE_DESCRIPTION("Qualcomm PCIe ECAM root complex driver");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.7.4
> > >
> >
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v3 21/25] drivers: media: i2c: imx258: Use macros
From: Luis Garcia @ 2024-04-06 5:21 UTC (permalink / raw)
To: Tommaso Merciai
Cc: Sakari Ailus, linux-media, dave.stevenson, jacopo.mondi, mchehab,
robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel, Ondrej Jirman
In-Reply-To: <ZhAGi0X2wVOdsrSe@tom-HP-ZBook-Fury-15-G7-Mobile-Workstation>
On 4/5/24 08:11, Tommaso Merciai wrote:
> Hi Luis,
>
> On Fri, Apr 05, 2024 at 04:33:38AM -0600, Luis Garcia wrote:
>> On 4/4/24 00:46, Sakari Ailus wrote:
>>> On Wed, Apr 03, 2024 at 01:17:26PM -0600, Luigi311 wrote:
>>>> On 4/3/24 10:23, Sakari Ailus wrote:
>>>>> Hi Luis,
>>>>>
>>>>> On Wed, Apr 03, 2024 at 09:03:50AM -0600, git@luigi311.com wrote:
>>>>>> From: Luis Garcia <git@luigi311.com>
>>>>>>
>>>>>> Use understandable macros instead of raw values.
>>>>>>
>>>>>> Signed-off-by: Ondrej Jirman <megi@xff.cz>
>>>>>> Signed-off-by: Luis Garcia <git@luigi311.com>
>>>>>> ---
>>>>>> drivers/media/i2c/imx258.c | 434 ++++++++++++++++++-------------------
>>>>>> 1 file changed, 207 insertions(+), 227 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>>>>>> index e2ecf6109516..30352c33f63c 100644
>>>>>> --- a/drivers/media/i2c/imx258.c
>>>>>> +++ b/drivers/media/i2c/imx258.c
>>>>>> @@ -33,8 +33,6 @@
>>>>>> #define IMX258_VTS_30FPS_VGA 0x034c
>>>>>> #define IMX258_VTS_MAX 65525
>>>>>>
>>>>>> -#define IMX258_REG_VTS 0x0340
>>>>>> -
>>>>>> /* HBLANK control - read only */
>>>>>> #define IMX258_PPL_DEFAULT 5352
>>>>>>
>>>>>> @@ -90,6 +88,53 @@
>>>>>> #define IMX258_PIXEL_ARRAY_WIDTH 4208U
>>>>>> #define IMX258_PIXEL_ARRAY_HEIGHT 3120U
>>>>>>
>>>>>> +/* regs */
>>>>>> +#define IMX258_REG_PLL_MULT_DRIV 0x0310
>>>>>> +#define IMX258_REG_IVTPXCK_DIV 0x0301
>>>>>> +#define IMX258_REG_IVTSYCK_DIV 0x0303
>>>>>> +#define IMX258_REG_PREPLLCK_VT_DIV 0x0305
>>>>>> +#define IMX258_REG_IOPPXCK_DIV 0x0309
>>>>>> +#define IMX258_REG_IOPSYCK_DIV 0x030b
>>>>>> +#define IMX258_REG_PREPLLCK_OP_DIV 0x030d
>>>>>> +#define IMX258_REG_PHASE_PIX_OUTEN 0x3030
>>>>>> +#define IMX258_REG_PDPIX_DATA_RATE 0x3032
>>>>>> +#define IMX258_REG_SCALE_MODE 0x0401
>>>>>> +#define IMX258_REG_SCALE_MODE_EXT 0x3038
>>>>>> +#define IMX258_REG_AF_WINDOW_MODE 0x7bcd
>>>>>> +#define IMX258_REG_FRM_LENGTH_CTL 0x0350
>>>>>> +#define IMX258_REG_CSI_LANE_MODE 0x0114
>>>>>> +#define IMX258_REG_X_EVN_INC 0x0381
>>>>>> +#define IMX258_REG_X_ODD_INC 0x0383
>>>>>> +#define IMX258_REG_Y_EVN_INC 0x0385
>>>>>> +#define IMX258_REG_Y_ODD_INC 0x0387
>>>>>> +#define IMX258_REG_BINNING_MODE 0x0900
>>>>>> +#define IMX258_REG_BINNING_TYPE_V 0x0901
>>>>>> +#define IMX258_REG_FORCE_FD_SUM 0x300d
>>>>>> +#define IMX258_REG_DIG_CROP_X_OFFSET 0x0408
>>>>>> +#define IMX258_REG_DIG_CROP_Y_OFFSET 0x040a
>>>>>> +#define IMX258_REG_DIG_CROP_IMAGE_WIDTH 0x040c
>>>>>> +#define IMX258_REG_DIG_CROP_IMAGE_HEIGHT 0x040e
>>>>>> +#define IMX258_REG_SCALE_M 0x0404
>>>>>> +#define IMX258_REG_X_OUT_SIZE 0x034c
>>>>>> +#define IMX258_REG_Y_OUT_SIZE 0x034e
>>>>>> +#define IMX258_REG_X_ADD_STA 0x0344
>>>>>> +#define IMX258_REG_Y_ADD_STA 0x0346
>>>>>> +#define IMX258_REG_X_ADD_END 0x0348
>>>>>> +#define IMX258_REG_Y_ADD_END 0x034a
>>>>>> +#define IMX258_REG_EXCK_FREQ 0x0136
>>>>>> +#define IMX258_REG_CSI_DT_FMT 0x0112
>>>>>> +#define IMX258_REG_LINE_LENGTH_PCK 0x0342
>>>>>> +#define IMX258_REG_SCALE_M_EXT 0x303a
>>>>>> +#define IMX258_REG_FRM_LENGTH_LINES 0x0340
>>>>>> +#define IMX258_REG_FINE_INTEG_TIME 0x0200
>>>>>> +#define IMX258_REG_PLL_IVT_MPY 0x0306
>>>>>> +#define IMX258_REG_PLL_IOP_MPY 0x030e
>>>>>> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H 0x0820
>>>>>> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L 0x0822
>>>>>> +
>>>>>> +#define REG8(a, v) { a, v }
>>>>>> +#define REG16(a, v) { a, ((v) >> 8) & 0xff }, { (a) + 1, (v) & 0xff }
>>>>>
>>>>> The patch is nice but these macros are better replaced by the V4L2 CCI
>>>>> helper that also offers register access functions. Could you add a patch to
>>>>> convert the driver to use it (maybe after this one)?
>>>>>
>>>>
>>>> Ohh perfect, using something else would be great. Ill go ahead and see
>>>> if I can get that working.
>>>
>>> Thanks. It may be easier to just do it in this one actually. Up to you.
>>>
>>
>> I've made the swap but looks like its not playing nice with my ppp,
>> its causing a crash and showing a call trace as soon as it does its
>> first read to check the identity. I went in and dropped the cci_read
>> and left it with the original implementation and I'm getting a very
>> similar crash with cci_write too so it looks like its not liking
>> how I'm implementing it. Looking at the few other drivers that were
>> swapped over to use that, I don't seem to be missing anything. It's
>> a big change so its not really something I can describe what I've
>> changed but I do have the change on my github here
>> https://github.com/luigi311/linux/commit/840593acb20eee87ce361e6929edf51eefbbe737
>
> I checked your commit to switch to cci helper.
> I think you are missing the right cci regmap initialization.
>
> Please take care to use: devm_cci_regmap_init_i2c
>
> /**
> * devm_cci_regmap_init_i2c() - Create regmap to use with cci_*() register
> * access functions
> *
> * @client: i2c_client to create the regmap for
> * @reg_addr_bits: register address width to use (8 or 16)
> *
> * Note the memory for the created regmap is devm() managed, tied to the client.
> *
> * Return: %0 on success or a negative error code on failure.
> */
>
> Check drivers/media/i2c/imx290.c:1530
> Hope this help :)
>
> Note:
> Somewhere into the github commit you are reading 16bit reg and storing
> that into 64bit val. Take care! :)
>
> Thanks & Regards,
> Tommaso
>
>
Ohh my god your right, looks like i missed that when i was replicating
the cci patch for that same driver lol. Looks like that fixed it and
the driver is now loading so i can continue to debug it and make sure
everything is set correct since i had a few that were messed up since
i couldn't test it.
>> if you can provide some guidance, if not I can skip this change
>> all together and we can do a separate attempt at swapping over to it.
>>
^ permalink raw reply
* Re: [PATCH v3 05/25] media: i2c: imx258: Add regulator control
From: Luis Garcia @ 2024-04-06 5:23 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2jgmmqw0nXDYcF@duo.ucw.cz>
On 4/3/24 12:44, Pavel Machek wrote:
> Hi!
>
>> The device tree bindings define the relevant regulators for the
>> sensor, so update the driver to request the regulators and control
>> them at the appropriate times.
>
>> @@ -995,9 +1007,19 @@ static int imx258_power_on(struct device *dev)
>> struct imx258 *imx258 = to_imx258(sd);
>> int ret;
>>
>> + ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
>> + imx258->supplies);
>> + if (ret) {
>
> Will this make it fail for all current users?
>
> Best regards,
> Pavel
>
It shouldn't affect current users as this was added in by dave for
a completely different sensor and it still works on my ppp. Looking
at the dmesg for imx258 it does reference the regulators that the
ppp doesnt have but it still works.
^ permalink raw reply
* Re: [PATCH v3 09/25] media: i2c: imx258: Add support for running on 2 CSI data lanes
From: Luis Garcia @ 2024-04-06 5:25 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2jvUDFnY83drlg@duo.ucw.cz>
On 4/3/24 12:45, Pavel Machek wrote:
> Hi!
>
>> +/*
>> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
>> + * To avoid further computation of clock settings, adopt the same per
>> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
>> + */
>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
>> + { 0x0136, 0x13 },
>> + { 0x0137, 0x33 },
>> + { 0x0301, 0x0A },
>> + { 0x0303, 0x02 },
>> + { 0x0305, 0x03 },
>> + { 0x0306, 0x00 },
>> + { 0x0307, 0xC6 },
>> + { 0x0309, 0x0A },
>> + { 0x030B, 0x01 },
>> + { 0x030D, 0x02 },
>> + { 0x030E, 0x00 },
>> + { 0x030F, 0xD8 },
>> + { 0x0310, 0x00 },
>> +
>> + { 0x0114, 0x01 },
>> + { 0x0820, 0x09 },
>> + { 0x0821, 0xa6 },
>> + { 0x0822, 0x66 },
>> + { 0x0823, 0x66 },
>> +};
>> +
>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
>> { 0x0136, 0x13 },
>> { 0x0137, 0x33 },
>> { 0x0301, 0x05 },
>
> I wish we did not have to copy all the magic values like this.
>
> Best regards,
> Pavel
>
no kidding, magic values everywhere.... it makes it annoying
for me to move things around because they all start to look
similar. Down the line we added in more defined names so its
not as bad but still its bad lol.
^ permalink raw reply
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
From: Vinod Koul @ 2024-04-06 6:19 UTC (permalink / raw)
To: Yang Xiwen
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
In-Reply-To: <SEZPR06MB695903034B7404FBEA750A9196032@SEZPR06MB6959.apcprd06.prod.outlook.com>
On 06-04-24, 01:53, Yang Xiwen wrote:
> On 4/6/2024 12:52 AM, Vinod Koul wrote:
> > On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
> > > From: Yang Xiwen <forbidden405@outlook.com>
> > That is quite an email id!
> >
> > > Direct MMIO resgiter access is used by Hi3798MV200. For other models,
> > > of_iomap() returns NULL due to insufficient length. So they are
> > so how is that fixed... Pls describe the change...
>
>
> The commit log will be rewritten in next revision. I'll try to emphasize the
> PHY and its configuration interface briefly. Though i don't have access to
> the datasheets and TRM so most things can not be verified.
>
>
> For CV200 and MV100 INNO PHY, the configuration interface is attached to
> PERICTRL(Peripheral Control Block). So we just use a register called
> PERI_USB3 to configure the PHY. The bus reset, clock are all controlled in
> PERI_USB3 register. To read/write to a register of the PHY, a special
> sequence of register writes and reads are needed, which was implemented in
> this driver.
>
>
> But for MV200 INNO PHY, the configuration interface is attached directly to
> system bus(MMIO). The bus clocks and resets are controlled via Clock Reset
> Generator(CRG). Now we have to control them with the help of linux clk and
> reset framework because they are provided by other modules.
Okay better log is welcome
>
>
> > > unaffected.
> > >
> > > Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
> > > switch to reset_control_array_*() APIs for that.
>
>
> The commit msg is misleading here. There is no extra reset actually. The
> reset also exist for existing users. The initial author just decided to
> manage it in the hisi_inno_phy_write_reg() routine(without using
> reset_control_* APIs) and omit it in the binding.
>
>
> > That probably should be a different patch
>
>
> I guess so. From my point of view, the whole patch is to introduce the
> support for Hi3798MV200 variant of the INNO PHY. So i've decided to squash
> the two changes into one single commit.
Not really you can build smaller reviewable changes leading up to adding
the Hi3798MV200 support
>
>
> >
> > > Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> > > ---
> > > drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 66 ++++++++++++++++++------------
> > > 1 file changed, 40 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> > > index b7e740eb4752..df154cd99ed8 100644
> > > --- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> > > +++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> > > @@ -10,6 +10,7 @@
> > > #include <linux/io.h>
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > +#include <linux/of_address.h>
> > > #include <linux/phy/phy.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/reset.h>
> > > @@ -24,6 +25,7 @@
> > > #define PHY_TYPE_0 0
> > > #define PHY_TYPE_1 1
> > > +#define PHY_TYPE_MMIO 2
> > > #define PHY_TEST_DATA GENMASK(7, 0)
> > > #define PHY_TEST_ADDR_OFFSET 8
> > > @@ -43,6 +45,7 @@
> > > #define PHY_CLK_ENABLE BIT(2)
> > > struct hisi_inno_phy_port {
> > > + void __iomem *base;
> > > struct reset_control *utmi_rst;
> > > struct hisi_inno_phy_priv *priv;
> > > };
> > > @@ -50,7 +53,7 @@ struct hisi_inno_phy_port {
> > > struct hisi_inno_phy_priv {
> > > void __iomem *mmio;
> > > struct clk *ref_clk;
> > > - struct reset_control *por_rst;
> > > + struct reset_control *rsts;
> > > unsigned int type;
> > > struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
> > > };
> > > @@ -62,26 +65,31 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
> > > u32 val;
> > > u32 value;
> > > - if (priv->type == PHY_TYPE_0)
> > > - val = (data & PHY_TEST_DATA) |
> > > - ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
> > > - ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
> > > - PHY0_TEST_WREN | PHY0_TEST_RST;
> > > - else
> > > - val = (data & PHY_TEST_DATA) |
> > > - ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
> > > - ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
> > > - PHY1_TEST_WREN | PHY1_TEST_RST;
> > > - writel(val, reg);
> > > -
> > > - value = val;
> > > - if (priv->type == PHY_TYPE_0)
> > > - value |= PHY0_TEST_CLK;
> > > - else
> > > - value |= PHY1_TEST_CLK;
> > > - writel(value, reg);
> > > -
> > > - writel(val, reg);
> > > + if (priv->ports[port].base)
> > > + /* FIXME: fill stride in priv */
> > when?
>
>
> I'm not sure. Maybe until some other users with stride other than 3? I don't
> have much knowledge about other SoCs.
>
>
> Maybe replace the FIXME here with some additional information.
Better
>
>
> >
> > > + writel(data, (u32 *)priv->ports[port].base + addr);
> > > + else {
> > > + if (priv->type == PHY_TYPE_0)
> > > + val = (data & PHY_TEST_DATA) |
> > > + ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
> > > + ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
> > > + PHY0_TEST_WREN | PHY0_TEST_RST;
> > > + else
> > > + val = (data & PHY_TEST_DATA) |
> > > + ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
> > > + ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
> > > + PHY1_TEST_WREN | PHY1_TEST_RST;
> > > + writel(val, reg);
> > > +
> > > + value = val;
> > > + if (priv->type == PHY_TYPE_0)
> > > + value |= PHY0_TEST_CLK;
> > > + else
> > > + value |= PHY1_TEST_CLK;
> > > + writel(value, reg);
> > > +
> > > + writel(val, reg);
> > val and value are very helpful variables, do consider naming them
> > better!
>
>
> I'll consider renaming them in the next revision. Maybe val and val2? They
> are just some temp vars to store register values.
Yeah, that might be better
>
>
> >
> > > + }
> > > }
> > > static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
> > > @@ -104,7 +112,7 @@ static int hisi_inno_phy_init(struct phy *phy)
> > > return ret;
> > > udelay(REF_CLK_STABLE_TIME);
> > > - reset_control_deassert(priv->por_rst);
> > > + reset_control_deassert(priv->rsts);
> > > udelay(POR_RST_COMPLETE_TIME);
> > > /* Set up phy registers */
> > > @@ -122,7 +130,7 @@ static int hisi_inno_phy_exit(struct phy *phy)
> > > struct hisi_inno_phy_priv *priv = port->priv;
> > > reset_control_assert(port->utmi_rst);
> > > - reset_control_assert(priv->por_rst);
> > > + reset_control_assert(priv->rsts);
> > > clk_disable_unprepare(priv->ref_clk);
> > > return 0;
> > > @@ -158,15 +166,16 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
> > > if (IS_ERR(priv->ref_clk))
> > > return PTR_ERR(priv->ref_clk);
> > > - priv->por_rst = devm_reset_control_get_exclusive(dev, NULL);
> > > - if (IS_ERR(priv->por_rst))
> > > - return PTR_ERR(priv->por_rst);
> > > + priv->rsts = devm_reset_control_array_get_exclusive(dev);
> > > + if (IS_ERR(priv->rsts))
> > > + return PTR_ERR(priv->rsts);
> > > priv->type = (uintptr_t) of_device_get_match_data(dev);
> > > for_each_child_of_node(np, child) {
> > > struct reset_control *rst;
> > > struct phy *phy;
> > > + void __iomem *base;
> > > rst = of_reset_control_get_exclusive(child, NULL);
> > > if (IS_ERR(rst)) {
> > > @@ -174,7 +183,10 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
> > > return PTR_ERR(rst);
> > > }
> > > + base = of_iomap(child, 0);
> > > +
> > > priv->ports[i].utmi_rst = rst;
> > > + priv->ports[i].base = base;
> > > priv->ports[i].priv = priv;
> > > phy = devm_phy_create(dev, child, &hisi_inno_phy_ops);
> > > @@ -205,6 +217,8 @@ static const struct of_device_id hisi_inno_phy_of_match[] = {
> > > .data = (void *) PHY_TYPE_0 },
> > > { .compatible = "hisilicon,hi3798mv100-usb2-phy",
> > > .data = (void *) PHY_TYPE_1 },
> > > + { .compatible = "hisilicon,hi3798mv200-usb2-phy",
> > > + .data = (void *) PHY_TYPE_MMIO },
> > > { },
> > > };
> > > MODULE_DEVICE_TABLE(of, hisi_inno_phy_of_match);
> > >
> > > --
> > > 2.43.0
>
>
> --
> Regards,
> Yang Xiwen
--
~Vinod
^ permalink raw reply
* [PATCH v6 0/2] riscv: pwm: sophgo: add pwm support for CV1800
From: Jingbao Qiu @ 2024-04-06 6:34 UTC (permalink / raw)
To: u.kleine-koenig, robh, krzysztof.kozlowski+dt, conor+dt,
inochiama, paul.walmsley, palmer, aou, linux-pwm, devicetree,
linux-kernel, linux-riscv
Cc: Jingbao Qiu
The Sophgo CV1800 chip provides a set of four independent
PWM channel outputs.
This series adds PWM controller support for Sophgo cv1800.
Changes since v5:
- delete the OE function because we plan to use the counter subsystem
instead of capture, so there is no need to reuse this code.
- fix set polarity reverse error.
v5: https://lore.kernel.org/all/20240314100131.323540-1-qiujingbao.dlmu@gmail.com/
Changes since v4:
- drop filename
- fix macro
- optimize cv1800_pwm_set_polarity()
- optimize cv1800_pwm_set_oe()
- add comment for cv1800_pwm_set_oe()
- use ticks replace tem
- fix duty_cycle larger than period_val
- use devm_clk_rate_exclusive_get() replace
clk_rate_exclusive_get()
- map linux polarity to register polarity
v4: https://lore.kernel.org/all/20240304085933.1246964-1-qiujingbao.dlmu@gmail.com/
datasheet Link: https://github.com/milkv-duo/duo-files/blob/main/duo/datasheet/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf
page 614
Changes since v3:
- use macro instead of npwm number
- add support for polarity feature
- add support for Output-Enable/OE feature
v3: https://lore.kernel.org/all/20240223082014.109385-1-qiujingbao.dlmu@gmail.com/
Changes since v2:
- use 0x08 instead of macro
- split if statements based on conditions
- in order to round up, first calculate the
number of high-level cycles, then subtract
it from the PERIOD to obtain the number of HLPERIOD
- use new pwmchip_alloc() API instead of old style
v2: https://lore.kernel.org/all/20240212121729.1086718-1-qiujingbao.dlmu@gmail.com/
Changes since v1:
- drop full stop from subject
- re-order maintainers and description
- pass checkpatch.pl --strict
- fix naming errors
- add "Limitations" section
- use a driver specific prefix for all defines
- using bool instead u32 in cv1800_pwm_enable
- check and set state->polarity
- use mul_u64_u64_div_u64
- use clk_rate_exclusive_get(), balance with clk_rate_exclusive_put()
- using macro definitions instead of shift operations
- remove shift operation on 0
- use priv replace cv_pwm
- hardcode npwm
- set atomic to true
- remove MODULE_ALIAS
v1: https://lore.kernel.org/all/20240207055856.672184-1-qiujingbao.dlmu@gmail.com/
Jingbao Qiu (2):
dt-bindings: pwm: sophgo: add pwm for Sophgo CV1800 series SoC
pwm: sophgo: add pwm support for Sophgo CV1800 SoC
.../bindings/pwm/sophgo,cv1800-pwm.yaml | 45 +++
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-cv1800.c | 296 ++++++++++++++++++
4 files changed, 352 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/sophgo,cv1800-pwm.yaml
create mode 100644 drivers/pwm/pwm-cv1800.c
--
2.25.1
^ permalink raw reply
* [PATCH v6 1/2] dt-bindings: pwm: sophgo: add pwm for Sophgo CV1800 series SoC
From: Jingbao Qiu @ 2024-04-06 6:34 UTC (permalink / raw)
To: u.kleine-koenig, robh, krzysztof.kozlowski+dt, conor+dt,
inochiama, paul.walmsley, palmer, aou, linux-pwm, devicetree,
linux-kernel, linux-riscv
Cc: Jingbao Qiu, Krzysztof Kozlowski
In-Reply-To: <20240406063413.3334639-1-qiujingbao.dlmu@gmail.com>
Add devicetree binding to describe the PWM for Sophgo CV1800 SoC.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
---
.../bindings/pwm/sophgo,cv1800-pwm.yaml | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/sophgo,cv1800-pwm.yaml
diff --git a/Documentation/devicetree/bindings/pwm/sophgo,cv1800-pwm.yaml b/Documentation/devicetree/bindings/pwm/sophgo,cv1800-pwm.yaml
new file mode 100644
index 000000000000..b5b819d780f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/sophgo,cv1800-pwm.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/sophgo,cv1800-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo CV1800 PWM controller
+
+maintainers:
+ - Jingbao Qiu <qiujingbao.dlmu@gmail.com>
+
+description:
+ The chip provides a set of four independent PWM channel outputs.
+
+allOf:
+ - $ref: pwm.yaml#
+
+properties:
+ compatible:
+ const: sophgo,cv1800-pwm
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 3
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ pwm0: pwm@3060000 {
+ compatible = "sophgo,cv1800-pwm";
+ reg = <0x3060000 0x1000>;
+ clocks = <&clk 60>;
+ #pwm-cells = <3>;
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v6 2/2] pwm: sophgo: add pwm support for Sophgo CV1800 SoC
From: Jingbao Qiu @ 2024-04-06 6:34 UTC (permalink / raw)
To: u.kleine-koenig, robh, krzysztof.kozlowski+dt, conor+dt,
inochiama, paul.walmsley, palmer, aou, linux-pwm, devicetree,
linux-kernel, linux-riscv
Cc: Jingbao Qiu
In-Reply-To: <20240406063413.3334639-1-qiujingbao.dlmu@gmail.com>
Implement the PWM driver for CV1800.
Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
---
drivers/pwm/Kconfig | 10 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-cv1800.c | 296 +++++++++++++++++++++++++++++++++++++++
3 files changed, 307 insertions(+)
create mode 100644 drivers/pwm/pwm-cv1800.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 1dd7921194f5..3869ca022aeb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -182,6 +182,16 @@ config PWM_CROS_EC
PWM driver for exposing a PWM attached to the ChromeOS Embedded
Controller.
+config PWM_CV1800
+ tristate "Sophgo CV1800 PWM driver"
+ depends on ARCH_SOPHGO || COMPILE_TEST
+ help
+ Generic PWM framework driver for the Sophgo CV1800 series
+ SoCs.
+
+ To compile this driver as a module, build the dependecies
+ as modules, this will be called pwm-cv1800.
+
config PWM_DWC_CORE
tristate
depends on HAS_IOMEM
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 90913519f11a..6295e2259efc 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_PWM_CLK) += pwm-clk.o
obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o
obj-$(CONFIG_PWM_CRC) += pwm-crc.o
obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o
+obj-$(CONFIG_PWM_CV1800) += pwm-cv1800.o
obj-$(CONFIG_PWM_DWC_CORE) += pwm-dwc-core.o
obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o
diff --git a/drivers/pwm/pwm-cv1800.c b/drivers/pwm/pwm-cv1800.c
new file mode 100644
index 000000000000..37a6be3f63aa
--- /dev/null
+++ b/drivers/pwm/pwm-cv1800.c
@@ -0,0 +1,296 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Sophgo CV1800 PWM driver
+ * Author: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
+ *
+ * Limitations:
+ * - It output low when PWM channel disabled.
+ * - This pwm device supports dynamic loading of PWM parameters. When PWMSTART
+ * is written from 0 to 1, the register value (HLPERIODn, PERIODn) will be
+ * temporarily stored inside the PWM. If you want to dynamically change the
+ * waveform during PWM output, after writing the new value to HLPERIODn and
+ * PERIODn, write 1 and then 0 to PWMUPDATE[n] to make the new value effective.
+ * - Supports up to Rate/2 output, and the lowest is about Rate/(2^30-1).
+ * - By setting HLPERIODn to 0, can produce 100% duty cycle.
+ * - This hardware could support inverted polarity. By default, the value of the
+ * POLARITY register is 0x0. This means that HLPERIOD represents the number
+ * of low level beats.
+ * - This hardware supports input mode and output mode, implemented through the
+ * Output-Enable/OE register. However, this driver has not yet implemented
+ * capture callback.
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+
+#define PWM_CV1800_HLPERIOD_BASE 0x00
+#define PWM_CV1800_PERIOD_BASE 0x04
+#define PWM_CV1800_POLARITY 0x40
+#define PWM_CV1800_START 0x44
+#define PWM_CV1800_DONE 0x48
+#define PWM_CV1800_UPDATE 0x4c
+#define PWM_CV1800_OE 0xd0
+
+#define PWM_CV1800_HLPERIOD(n) (PWM_CV1800_HLPERIOD_BASE + ((n)*0x08))
+#define PWM_CV1800_PERIOD(n) (PWM_CV1800_PERIOD_BASE + ((n)*0x08))
+
+#define PWM_CV1800_UPDATE_MASK(n) (BIT(0) << (n))
+#define PWM_CV1800_OE_MASK(n) (BIT(0) << (n))
+#define PWM_CV1800_START_MASK(n) (BIT(0) << (n))
+#define PWM_CV1800_POLARITY_MASK(n) (BIT(0) << (n))
+
+#define PWM_CV1800_MAXPERIOD 0x3fffffff
+#define PWM_CV1800_MINPERIOD 2
+#define PWM_CV1800_CHANNELS 4
+#define PWM_CV1800_PERIOD_RESET BIT(1)
+#define PWM_CV1800_HLPERIOD_RESET BIT(0)
+#define PWM_CV1800_REG_DISABLE 0x00U
+#define PWM_CV1800_REG_ENABLE(n) (BIT(0) << (n))
+
+struct cv1800_pwm {
+ struct regmap *map;
+ struct clk *clk;
+ unsigned long clk_rate;
+};
+
+static inline struct cv1800_pwm *to_cv1800_pwm_dev(struct pwm_chip *chip)
+{
+ return pwmchip_get_drvdata(chip);
+}
+
+static const struct regmap_config cv1800_pwm_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
+static int cv1800_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
+ bool enable)
+{
+ struct cv1800_pwm *priv = to_cv1800_pwm_dev(chip);
+ u32 pwm_enable, state;
+
+ regmap_read(priv->map, PWM_CV1800_START, &pwm_enable);
+ pwm_enable &= PWM_CV1800_START_MASK(pwm->hwpwm);
+
+ /*
+ * If the parameters are changed during runtime, Register needs
+ * to be updated to take effect.
+ */
+ if (pwm_enable && enable) {
+ regmap_update_bits(priv->map, PWM_CV1800_UPDATE,
+ PWM_CV1800_UPDATE_MASK(pwm->hwpwm),
+ PWM_CV1800_REG_ENABLE(pwm->hwpwm));
+ regmap_update_bits(priv->map, PWM_CV1800_UPDATE,
+ PWM_CV1800_UPDATE_MASK(pwm->hwpwm),
+ PWM_CV1800_REG_DISABLE);
+ } else if (!pwm_enable && enable) {
+ regmap_update_bits(priv->map, PWM_CV1800_START,
+ PWM_CV1800_START_MASK(pwm->hwpwm),
+ PWM_CV1800_REG_ENABLE(pwm->hwpwm));
+ } else if (pwm_enable && !enable) {
+ regmap_update_bits(priv->map, PWM_CV1800_START,
+ PWM_CV1800_START_MASK(pwm->hwpwm),
+ PWM_CV1800_REG_DISABLE);
+ }
+
+ /* check and set OE/Output-Enable mode */
+ regmap_read(priv->map, PWM_CV1800_OE, &state);
+ state &= PWM_CV1800_OE_MASK(pwm->hwpwm);
+
+ if (state == PWM_CV1800_REG_DISABLE && enable)
+ regmap_update_bits(priv->map, PWM_CV1800_OE,
+ PWM_CV1800_OE_MASK(pwm->hwpwm),
+ PWM_CV1800_REG_ENABLE(pwm->hwpwm));
+
+ return 0;
+}
+
+static void cv1800_pwm_set_polarity(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ enum pwm_polarity polarity)
+{
+ struct cv1800_pwm *priv = to_cv1800_pwm_dev(chip);
+ u32 config_polarity = 0;
+
+ if (pwm->state.enabled)
+ cv1800_pwm_enable(chip, pwm, !pwm->state.enabled);
+
+ if (polarity == PWM_POLARITY_NORMAL)
+ config_polarity = PWM_CV1800_POLARITY_MASK(pwm->hwpwm);
+
+ regmap_update_bits(priv->map, PWM_CV1800_POLARITY,
+ PWM_CV1800_POLARITY_MASK(pwm->hwpwm),
+ config_polarity);
+}
+
+static int cv1800_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct cv1800_pwm *priv = to_cv1800_pwm_dev(chip);
+ u32 period_val, hlperiod_val;
+ u64 ticks;
+ u32 state;
+
+ if (state->polarity != pwm->state.polarity)
+ cv1800_pwm_set_polarity(chip, pwm, state->polarity);
+
+ /*
+ * This hardware use PERIOD and HLPERIOD registers to represent PWM waves.
+ *
+ * The meaning of PERIOD is how many clock cycles (from the clock source)
+ * are used to represent PWM waves.
+ * PERIOD = rate(MHz) / target(MHz)
+ * PERIOD = period(ns) * rate(Hz) / NSEC_PER_SEC
+ */
+ ticks = mul_u64_u64_div_u64(state->period, priv->clk_rate,
+ NSEC_PER_SEC);
+ if (ticks < PWM_CV1800_MINPERIOD)
+ return -EINVAL;
+
+ if (ticks > PWM_CV1800_MAXPERIOD)
+ ticks = PWM_CV1800_MAXPERIOD;
+ period_val = (u32)ticks;
+
+ /*
+ * After mapping, hlperiod represents the same polarity as duty.
+ * HLPERIOD = rate(MHz) / duty(MHz)
+ * HLPERIOD = duty(ns) * rate(Hz) / NSEC_PER_SEC
+ */
+ ticks = mul_u64_u64_div_u64(state->duty_cycle, priv->clk_rate,
+ NSEC_PER_SEC);
+ if (ticks > period_val)
+ ticks = period_val;
+ hlperiod_val = (u32)ticks;
+
+ regmap_write(priv->map, PWM_CV1800_PERIOD(pwm->hwpwm), period_val);
+ regmap_write(priv->map, PWM_CV1800_HLPERIOD(pwm->hwpwm), hlperiod_val);
+
+ cv1800_pwm_enable(chip, pwm, state->enabled);
+
+ return 0;
+}
+
+static int cv1800_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct cv1800_pwm *priv = to_cv1800_pwm_dev(chip);
+ u32 period_val, hlperiod_val;
+ u64 period_ns = 0, duty_ns = 0;
+ u32 enable = 0, polarity = 0;
+
+ regmap_read(priv->map, PWM_CV1800_PERIOD(pwm->hwpwm), &period_val);
+ regmap_read(priv->map, PWM_CV1800_HLPERIOD(pwm->hwpwm), &hlperiod_val);
+
+ if (period_val != PWM_CV1800_PERIOD_RESET ||
+ hlperiod_val != PWM_CV1800_HLPERIOD_RESET) {
+ period_ns = DIV_ROUND_UP_ULL(period_val * NSEC_PER_SEC,
+ priv->clk_rate);
+ duty_ns = DIV_ROUND_UP_ULL(hlperiod_val * NSEC_PER_SEC,
+ priv->clk_rate);
+
+ regmap_read(priv->map, PWM_CV1800_START, &enable);
+ enable &= PWM_CV1800_START_MASK(pwm->hwpwm);
+
+ regmap_read(priv->map, PWM_CV1800_POLARITY, &polarity);
+ polarity &= PWM_CV1800_POLARITY_MASK(pwm->hwpwm);
+ }
+
+ state->period = period_ns;
+ state->duty_cycle = duty_ns;
+ state->enabled = enable;
+
+ /*
+ * To ensure that duty and hlperiod represent the same polarity
+ * the following mapping needs to be completed.
+ *
+ * |----------|------------|------------|-----------|
+ * | Linux | register | duty | register |
+ * | polarity | polarity | | hlperiod |
+ * |----------|------------|------------|-----------|
+ * | 1 | 0 | low level | low level |
+ * |----------|------------|------------|-----------|
+ * | 0 | 1 | high level | high level|
+ * |----------|------------|------------|-----------|
+ */
+ state->polarity = polarity ? PWM_POLARITY_NORMAL :
+ PWM_POLARITY_INVERSED;
+
+ return 0;
+}
+
+static const struct pwm_ops cv1800_pwm_ops = {
+ .apply = cv1800_pwm_apply,
+ .get_state = cv1800_pwm_get_state,
+};
+
+static int cv1800_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct cv1800_pwm *priv;
+ struct pwm_chip *chip;
+ void __iomem *base;
+ int ret;
+
+ chip = devm_pwmchip_alloc(dev, PWM_CV1800_CHANNELS, sizeof(*priv));
+ if (!chip)
+ return PTR_ERR(chip);
+ priv = to_cv1800_pwm_dev(chip);
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ priv->map = devm_regmap_init_mmio(&pdev->dev, base,
+ &cv1800_pwm_regmap_config);
+ if (IS_ERR(priv->map))
+ return PTR_ERR(priv->map);
+
+ priv->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+ if (IS_ERR(priv->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(priv->clk),
+ "clk not found\n");
+
+ ret = devm_clk_rate_exclusive_get(dev, priv->clk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to get exclusive rate\n");
+
+ priv->clk_rate = clk_get_rate(priv->clk);
+ if (!priv->clk_rate)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid clock rate: %lu\n",
+ priv->clk_rate);
+
+ chip->ops = &cv1800_pwm_ops;
+
+ ret = devm_pwmchip_add(dev, chip);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add PWM chip\n");
+
+ return 0;
+}
+
+static const struct of_device_id cv1800_pwm_dt_ids[] = {
+ { .compatible = "sophgo,cv1800-pwm" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, cv1800_pwm_dt_ids);
+
+static struct platform_driver cv1800_pwm_driver = {
+ .probe = cv1800_pwm_probe,
+ .driver = {
+ .name = "cv1800-pwm",
+ .of_match_table = cv1800_pwm_dt_ids,
+ },
+};
+module_platform_driver(cv1800_pwm_driver);
+
+MODULE_AUTHOR("Jingbao Qiu");
+MODULE_DESCRIPTION("Sophgo cv1800 PWM Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 11/25] media: i2c: imx258: Add get_selection for pixel array information
From: Luis Garcia @ 2024-04-06 6:36 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2kHYc6kdiZEAFD@duo.ucw.cz>
On 4/3/24 12:46, Pavel Machek wrote:
> Hi!
>
>> Libcamera requires the cropping information for each mode, so
>> add this information to the driver.
>
>> @@ -116,6 +124,9 @@ struct imx258_mode {
>> u32 link_freq_index;
>> /* Default register values */
>> struct imx258_reg_list reg_list;
>> +
>> + /* Analog crop rectangle. */
>
> No need for "." at the end, as it is not above.
>
Done
>> + struct v4l2_rect crop;
>> };
>
> If the crop is same in all modes, should we have it in common place?
>
> Best regards,
> Pavel
I see the imx219 made a similar change where it was changed from using
that .crop format to just setting it in the set_pad_format function
so we can do something similar here.
^ permalink raw reply
* Re: [PATCH v4 02/10] phy: rockchip: add usbdp combo phy driver
From: Vinod Koul @ 2024-04-06 7:06 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Heiko Stuebner, Kishon Vijay Abraham I, linux-rockchip, linux-phy,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frank Wang,
Kever Yang, devicetree, linux-kernel, kernel, Zhang Yubing
In-Reply-To: <20240325141653.84910-3-sebastian.reichel@collabora.com>
On 25-03-24, 15:15, Sebastian Reichel wrote:
> This adds a new USBDP combo PHY with Samsung IP block driver.
>
> The driver get lane mux and mapping info in 2 ways, supporting
> DisplayPort alternate mode or parsing from DT. When parsing from DT,
> the property "rockchip,dp-lane-mux" provide the DP mux and mapping
> info. This is needed when the PHY is not used with TypeC Alt-Mode.
> For example if the USB3 interface of the PHY is connected to a USB
> Type A connector and the DP interface is connected to a DisplayPort
> connector.
>
> When do DP link training, need to set lane number, link rate, swing,
> and pre-emphasis via PHY configure interface.
>
> Co-developed-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Co-developed-by: Zhang Yubing <yubing.zhang@rock-chips.com>
> Signed-off-by: Zhang Yubing <yubing.zhang@rock-chips.com>
> Co-developed-by: Frank Wang <frank.wang@rock-chips.com>
> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/phy/rockchip/Kconfig | 12 +
> drivers/phy/rockchip/Makefile | 1 +
> drivers/phy/rockchip/phy-rockchip-usbdp.c | 1612 +++++++++++++++++++++
> 3 files changed, 1625 insertions(+)
> create mode 100644 drivers/phy/rockchip/phy-rockchip-usbdp.c
>
> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> index a34f67bb7e61..c3d62243b474 100644
> --- a/drivers/phy/rockchip/Kconfig
> +++ b/drivers/phy/rockchip/Kconfig
> @@ -115,3 +115,15 @@ config PHY_ROCKCHIP_USB
> select GENERIC_PHY
> help
> Enable this to support the Rockchip USB 2.0 PHY.
> +
> +config PHY_ROCKCHIP_USBDP
> + tristate "Rockchip USBDP COMBO PHY Driver"
> + depends on ARCH_ROCKCHIP && OF
> + select GENERIC_PHY
> + select TYPEC
> + help
> + Enable this to support the Rockchip USB3.0/DP combo PHY with
> + Samsung IP block. This is required for USB3 support on RK3588.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called phy-rockchip-usbdp
> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
> index 3d911304e654..010a824e32ce 100644
> --- a/drivers/phy/rockchip/Makefile
> +++ b/drivers/phy/rockchip/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_PHY_ROCKCHIP_SAMSUNG_HDPTX) += phy-rockchip-samsung-hdptx.o
> obj-$(CONFIG_PHY_ROCKCHIP_SNPS_PCIE3) += phy-rockchip-snps-pcie3.o
> obj-$(CONFIG_PHY_ROCKCHIP_TYPEC) += phy-rockchip-typec.o
> obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
> +obj-$(CONFIG_PHY_ROCKCHIP_USBDP) += phy-rockchip-usbdp.o
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> new file mode 100644
> index 000000000000..38dc96cfe403
> --- /dev/null
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -0,0 +1,1612 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Rockchip USBDP Combo PHY with Samsung IP block driver
> + *
> + * Copyright (C) 2021 Rockchip Electronics Co., Ltd
we are in 2024!
> + */
> +
> +#include <dt-bindings/phy/phy.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
Do you need both
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/typec_mux.h>
That seems a bit more of headers, do you need all of them?
> +static int rk_udphy_setup(struct rk_udphy *udphy)
> +{
> + int ret = 0;
Superfluous init
> +static int rk_udphy_parse_dt(struct rk_udphy *udphy)
> +{
> + struct device *dev = udphy->dev;
> + struct device_node *np = dev_of_node(dev);
> + enum usb_device_speed maximum_speed;
> + int ret;
> +
> + udphy->u2phygrf = syscon_regmap_lookup_by_phandle(np, "rockchip,u2phy-grf");
> + if (IS_ERR(udphy->u2phygrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->u2phygrf), "failed to get u2phy-grf\n");
> +
> + udphy->udphygrf = syscon_regmap_lookup_by_phandle(np, "rockchip,usbdpphy-grf");
> + if (IS_ERR(udphy->udphygrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->udphygrf), "failed to get usbdpphy-grf\n");
> +
> + udphy->usbgrf = syscon_regmap_lookup_by_phandle(np, "rockchip,usb-grf");
> + if (IS_ERR(udphy->usbgrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->usbgrf), "failed to get usb-grf\n");
> +
> + udphy->vogrf = syscon_regmap_lookup_by_phandle(np, "rockchip,vo-grf");
> + if (IS_ERR(udphy->vogrf))
> + return dev_err_probe(dev, PTR_ERR(udphy->vogrf), "failed to get vo-grf\n");
> +
> + ret = rk_udphy_parse_lane_mux_data(udphy);
> + if (ret)
> + return ret;
> +
> + udphy->sbu1_dc_gpio = devm_gpiod_get_optional(dev, "sbu1-dc", GPIOD_OUT_LOW);
> + if (IS_ERR(udphy->sbu1_dc_gpio))
> + return PTR_ERR(udphy->sbu1_dc_gpio);
> +
> + udphy->sbu2_dc_gpio = devm_gpiod_get_optional(dev, "sbu2-dc", GPIOD_OUT_LOW);
> + if (IS_ERR(udphy->sbu2_dc_gpio))
> + return PTR_ERR(udphy->sbu2_dc_gpio);
> +
> + if (device_property_present(dev, "maximum-speed")) {
> + maximum_speed = usb_get_maximum_speed(dev);
> + udphy->hs = maximum_speed <= USB_SPEED_HIGH ? true : false;
> + }
> +
> + ret = rk_udphy_clk_init(udphy, dev);
> + if (ret)
> + return ret;
> +
> + ret = rk_udphy_reset_init(udphy, dev);
> + if (ret)
> + return ret;
> +
> + return 0;
return rk_udphy_reset_init()
> +static const struct phy_ops rk_udphy_dp_phy_ops = {
> + .init = rk_udphy_dp_phy_init,
> + .exit = rk_udphy_dp_phy_exit,
> + .power_on = rk_udphy_dp_phy_power_on,
> + .power_off = rk_udphy_dp_phy_power_off,
> + .configure = rk_udphy_dp_phy_configure,
> + .owner = THIS_MODULE,
> +};
> +
> +static int rk_udphy_usb3_phy_init(struct phy *phy)
> +{
> + struct rk_udphy *udphy = phy_get_drvdata(phy);
> + int ret = 0;
Superfluous init here too
--
~Vinod
^ permalink raw reply
* Re: (subset) [PATCH 0/2] phy: qcom-qmp-ufs: Fix PHY QMP clocks for SC7180
From: Vinod Koul @ 2024-04-06 9:19 UTC (permalink / raw)
To: andersson, konrad.dybcio, kishon, robh, krzysztof.kozlowski+dt,
conor+dt, cros-qcom-dts-watchers, manivannan.sadhasivam,
davidwronek, Danila Tikhonov
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20240401182240.55282-1-danila@jiaxyga.com>
On Mon, 01 Apr 2024 21:22:38 +0300, Danila Tikhonov wrote:
> This series of patches is based on the series from Manivannan:
> https://lore.kernel.org/all/20240131-ufs-phy-clock-v3-0-58a49d2f4605@linaro.org/
>
> Patch from David adding a UFS nodes for SC7180(SM7125):
> https://lore.kernel.org/all/20240121-sm7125-upstream-v4-6-f7d1212c8ebb@gmail.com/
>
> The patch submitted by David and a series of patches submitted by Manivannan
> were both applied at approximately the same time. As a result, David's patch
> did not include this change.
>
> [...]
Applied, thanks!
[1/2] dt-bindings: phy: qmp-ufs: Fix PHY clocks for SC7180
commit: 7c1f42967b75bdcd0640c52d37d58d8dd122989b
Best regards,
--
~Vinod
^ permalink raw reply
* Re: (subset) [PATCH 00/17] HSI2, UFS & UFS phy support for Tensor GS101
From: Vinod Koul @ 2024-04-06 9:19 UTC (permalink / raw)
To: mturquette, sboyd, robh, krzk+dt, conor+dt, kishon, alim.akhtar,
avri.altman, bvanassche, s.nawrocki, cw00.choi, jejb,
martin.petersen, chanho61.park, ebiggers, Peter Griffin
Cc: linux-scsi, linux-phy, devicetree, linux-clk, linux-samsung-soc,
linux-kernel, linux-arm-kernel, tudor.ambarus, andre.draszik,
saravanak, willmcvicker
In-Reply-To: <20240404122559.898930-1-peter.griffin@linaro.org>
On Thu, 04 Apr 2024 13:25:42 +0100, Peter Griffin wrote:
> This series adds support for the High Speed Interface (HSI) 2 clock
> management unit, UFS controller and UFS phy calibration/tuning for GS101.
>
> With this series applied, UFS is now functional! The SKhynix HN8T05BZGKX015
> can be enumerated, partitions mounted etc. This then allows us to move away
> from the initramfs rootfs we have been using for development so far.
>
> [...]
Applied, thanks!
[04/17] dt-bindings: phy: samsung,ufs-phy: Add dedicated gs101-ufs-phy compatible
commit: 724e4fc053fe217d0ed477517ae68db11feab1f5
[09/17] phy: samsung-ufs: use exynos_get_pmu_regmap_by_phandle() to obtain PMU regmap
commit: f2c6d0fa197a1558f4ef50162bb87e6644af232d
[10/17] phy: samsung-ufs: ufs: Add SoC callbacks for calibration and clk data recovery
commit: a4de58a9096b471f9dc1c2bc6bfaa8aa48110c31
[11/17] phy: samsung-ufs: ufs: Add support for gs101 UFS phy tuning
commit: c1cf725db1065153459f0deb69bd4d497a5fd183
[17/17] MAINTAINERS: Add phy-gs101-ufs file to Tensor GS101.
commit: 0338e1d2f933a4ec7ae96ed1f40c39b899e357d7
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH v5 7/7] iio: accel: adxl345: Add spi-3wire option
From: Jonathan Cameron @ 2024-04-06 10:15 UTC (permalink / raw)
To: Lothar Rubusch
Cc: Jonathan Cameron, lars, Michael.Hennerich, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
linux-kernel, eraretuya
In-Reply-To: <CAFXKEHYrqA2iaQJeMqPbQJYEu10xiRt92c-ySGcfH76MSriufw@mail.gmail.com>
On Tue, 2 Apr 2024 12:11:43 +0200
Lothar Rubusch <l.rubusch@gmail.com> wrote:
> On Mon, Apr 1, 2024 at 6:53 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Mon, 1 Apr 2024 18:06:24 +0200
> > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
> > > On Sat, Mar 30, 2024 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > >
> > > > On Fri, 29 Mar 2024 01:33:01 +0100
> > > > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> > > >
> > > > > On Thu, Mar 28, 2024 at 2:39 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > > > >
> > > > > > On Wed, 27 Mar 2024 22:03:20 +0000
> > > > > > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> > > > > >
> > > > > > > Add a setup function implementation to the spi module to enable spi-3wire
> > > > > > > as option when specified in the device-tree.
> > > > > > >
> > > > > > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > > > > > > ---
> > > > > > > drivers/iio/accel/adxl345.h | 2 ++
> > > > > > > drivers/iio/accel/adxl345_spi.c | 12 +++++++++++-
> > > > > > > 2 files changed, 13 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> > > > > > > index 4ea9341d4..e6bc3591c 100644
> > > > > > > --- a/drivers/iio/accel/adxl345.h
> > > > > > > +++ b/drivers/iio/accel/adxl345.h
> > > > > > > @@ -30,6 +30,8 @@
> > > > > > > #define ADXL345_POWER_CTL_MEASURE BIT(3)
> > > > > > > #define ADXL345_POWER_CTL_STANDBY 0x00
> > > > > > >
> > > > > > > +#define ADXL345_DATA_FORMAT_SPI_3WIRE BIT(6) /* 3-wire SPI mode */
> > > > > > > +
> > > > > > > #define ADXL345_DATA_FORMAT_RANGE GENMASK(1, 0) /* Set the g range */
> > > > > > > #define ADXL345_DATA_FORMAT_JUSTIFY BIT(2) /* Left-justified (MSB) mode */
> > > > > > > #define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits resolution */
> > > > > > > diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> > > > > > > index 1c0513bd3..f145d5c1d 100644
> > > > > > > --- a/drivers/iio/accel/adxl345_spi.c
> > > > > > > +++ b/drivers/iio/accel/adxl345_spi.c
> > > > > > > @@ -20,6 +20,16 @@ static const struct regmap_config adxl345_spi_regmap_config = {
> > > > > > > .read_flag_mask = BIT(7) | BIT(6),
> > > > > > > };
> > > > > > >
> > > > > > > +static int adxl345_spi_setup(struct device *dev, struct regmap *regmap)
> > > > > > > +{
> > > > > > > + struct spi_device *spi = container_of(dev, struct spi_device, dev);
> > > > > > > +
> > > > > > > + if (spi->mode & SPI_3WIRE)
> > > > > > > + return regmap_write(regmap, ADXL345_REG_DATA_FORMAT,
> > > > > > > + ADXL345_DATA_FORMAT_SPI_3WIRE);
> > > > > > Your earlier patch carefully (I think) left one or two fields alone, then
> > > > > > this write just comes in and changes them. In particular INT_INVERT.
> > > > > >
> > > > > Why do you refer here to INT_INVERT? In this code above I try to set
> > > > > SPI to 3 lines. Since this is a SPI configuration, i.e. bus specific,
> > > > > it happens in adxl345_spi.c. Passing this function to the bus
> > > > > independent adxl345_core.c file allows to configure the bus first.
> > > > > Therefore, I'm using the update function in core masking out the SPI
> > > > > filag.
> > > >
> > > > Ah. Ok. It was only intended to mask out the SPI3-wire bit, not the
> > > > other bits that you also masked out. I thought intent was to leave
> > > > them untouched for some reason. Given they don't matter in the driver
> > > > at the moment (no interrupt support) then no problem.
> > > >
> > > > >
> > > > > My reason why I try to keep INT_INVERT out is different. There is
> > > > > another driver for the same part in the kernel:
> > > > > ./drivers/input/misc/adxl34x.c - This is a input driver, using the
> > > > > interrupts of the adxl345 for the input device implementation. I
> > > > > assumed, that in the iio implementation there won't be interrupt
> > > > > handling for the adx345, since it is not an input device. Does this
> > > > > make sense?
> > > >
> > > > No. You can't use these two drivers at the same time. They will almost
> > > > certainly trample over each other in actually reading channels etc.
> > > >
> > > > Their is some legacy of old drivers in input from a long time back.
> > > > Given this driver clearly doesn't have full functionality yet in IIO there
> > > > and the different userspace ABI, we've just left the input driver alone.
> > > >
> > > Going by the git history gave this impression, too. But it was still a
> > > bit confusing to me.
> > >
> > > The IIO driver so far does not handle any of the interrupt features.
> > > The older driver also seems to support more of the chip's features.
> > > Would it make sense to continue implement/port what's missing -
> > > feature by feature - for the IIO driver in order to make the input
> > > driver obsolete (one day)?
> >
> > Yes, though it will be challenging because of the ABI differences.
> > We do have a few cross subsystem bridge drivers, but the few goes we've
> > had at an accel bridge driver haven't made it into the kernel. In particular
> > we don't have an in kernel interface for threshold events and similar in IIO.
> > It would be easy enough to add one, but no one has ever cared enough to
> > do the work. :(
> >
> Perhaps there are easier things (precision, power saving measures,
> etc) of that particular accelerometer to port first. Open issues which
> even I could give a try here. Sounds really exciting to me, though,
> but before I definitely need a bit more experience with community and
> APIs.
That would be great.
>
> What do you mean with cross subsystem bridge drivers, or this accel
> bridge driver? I mean, can you provide me with a link to that driver
> to have a look into what direction that is going to?
The last go at this was probably this one.
https://lore.kernel.org/linux-iio/d52cf9ee5944c90c69f6e74a46d844cef51e487e.1555362312.git.hns@goldelico.com/
>
> Anyway, I really appreciate already your patience with my patches, the
> direct and helpful answers! I appreciate the support. Thinking and
> re-thinking over every particular line of code is really an
> experience. I don't want to go too much into off-topic discussions
> here, if this is not the forum for that, please let me know :)
People used to sometimes be on IRC but I'm not sure anyone is any more :(
I rarely joined because of network restrictions at work.
It's fine to ask questions on this mailing list though.
Nature of kernel development is that people need to get good at skipping
threads they aren't interested in!
Jonathan
>
> > Jonathan
> >
> >
> > >
> > > > >
> > > > > > If it doesn't makes sense to write it there, either write that bit
> > > > > > every time here, or leave it alone every time. Not decide on whether
> > > > > > to write the bit based on SPI_3WIRE or not. As far as I know they
> > > > > > are unconnected features.
> > > > > >
> > > > > I think I did not understand. Could you please specify a bit more?
> > > > > When spi-3wire is configured in the DT it has to be parsed and handled
> > > > > in the bus specific part, i.e. the adxl345_spi.c Therefore I configure
> > > > > SPI_3WIRE there. I don't want to place SPI specific code in the core
> > > > > file.
> > > >
> > > > My confusion was that you were deliberately not touching the other unused
> > > > flags. In reality you are touching the but only if you enable 3wire.
> > > > I would write them register to 0 in the !3wire case so all other values
> > > > are the same in both paths.
> > > >
> > > > >
> > > > > > > + return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > static int adxl345_spi_probe(struct spi_device *spi)
> > > > > > > {
> > > > > > > struct regmap *regmap;
> > > > > > > @@ -33,7 +43,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
> > > > > > > if (IS_ERR(regmap))
> > > > > > > return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
> > > > > > >
> > > > > > > - return adxl345_core_probe(&spi->dev, regmap, NULL);
> > > > > > > + return adxl345_core_probe(&spi->dev, regmap, adxl345_spi_setup);
> > > > > > > }
> > > > > > >
> > > > > > > static const struct adxl345_chip_info adxl345_spi_info = {
> > > > > >
> > > >
> > >
> >
^ permalink raw reply
* Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
From: kernel test robot @ 2024-04-06 10:32 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Rob Clark, Abhinav Kumar,
Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: llvm, oe-kbuild-all, linux-arm-msm, linux-kernel, dri-devel,
freedreno, devicetree, Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-4-ce2b864251b1@linaro.org>
Hi Konrad,
kernel test robot noticed the following build errors:
[auto build test ERROR on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link: https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-4-ce2b864251b1%40linaro.org
patch subject: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
config: i386-buildonly-randconfig-001-20240406 (https://download.01.org/0day-ci/archive/20240406/202404061839.0waGfXwj-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240406/202404061839.0waGfXwj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404061839.0waGfXwj-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:1090:14: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1090 | *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
| ^
1 error generated.
vim +/FIELD_PREP +1090 drivers/gpu/drm/msm/adreno/adreno_gpu.c
1062
1063 int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
1064 struct device *dev, u32 *speedbin)
1065 {
1066 u32 fcode, pcode;
1067 int ret;
1068
1069 /* Try reading the speedbin via a nvmem cell first */
1070 ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
1071 if (!ret && ret != -EINVAL)
1072 return ret;
1073
1074 ret = qcom_smem_get_feature_code(&fcode);
1075 if (ret) {
1076 dev_err(dev, "Couldn't get feature code from SMEM!\n");
1077 return ret;
1078 }
1079
1080 ret = qcom_smem_get_product_code(&pcode);
1081 if (ret) {
1082 dev_err(dev, "Couldn't get product code from SMEM!\n");
1083 return ret;
1084 }
1085
1086 /* Don't consider fcode for external feature codes */
1087 if (fcode <= SOCINFO_FC_EXT_RESERVE)
1088 fcode = SOCINFO_FC_UNKNOWN;
1089
> 1090 *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
1091 FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
1092
1093 return ret;
1094 }
1095
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
From: kernel test robot @ 2024-04-06 10:42 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Rob Clark, Abhinav Kumar,
Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: oe-kbuild-all, linux-arm-msm, linux-kernel, dri-devel, freedreno,
devicetree, Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-4-ce2b864251b1@linaro.org>
Hi Konrad,
kernel test robot noticed the following build errors:
[auto build test ERROR on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link: https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-4-ce2b864251b1%40linaro.org
patch subject: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
config: i386-buildonly-randconfig-003-20240406 (https://download.01.org/0day-ci/archive/20240406/202404061841.njUovDV7-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240406/202404061841.njUovDV7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404061841.njUovDV7-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/adreno/adreno_gpu.c: In function 'adreno_read_speedbin':
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:1090:14: error: implicit declaration of function 'FIELD_PREP'; did you mean 'NEED_PGE'? [-Werror=implicit-function-declaration]
*speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
^~~~~~~~~~
NEED_PGE
cc1: some warnings being treated as errors
vim +1090 drivers/gpu/drm/msm/adreno/adreno_gpu.c
1062
1063 int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
1064 struct device *dev, u32 *speedbin)
1065 {
1066 u32 fcode, pcode;
1067 int ret;
1068
1069 /* Try reading the speedbin via a nvmem cell first */
1070 ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
1071 if (!ret && ret != -EINVAL)
1072 return ret;
1073
1074 ret = qcom_smem_get_feature_code(&fcode);
1075 if (ret) {
1076 dev_err(dev, "Couldn't get feature code from SMEM!\n");
1077 return ret;
1078 }
1079
1080 ret = qcom_smem_get_product_code(&pcode);
1081 if (ret) {
1082 dev_err(dev, "Couldn't get product code from SMEM!\n");
1083 return ret;
1084 }
1085
1086 /* Don't consider fcode for external feature codes */
1087 if (fcode <= SOCINFO_FC_EXT_RESERVE)
1088 fcode = SOCINFO_FC_UNKNOWN;
1089
> 1090 *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
1091 FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
1092
1093 return ret;
1094 }
1095
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox