[PATCH 2/2] virtio-pci: Do not break the error message for VIRTIO_F_VERSION_1
Breaking the error message will make it harder to grep for it in the
driver. So let's put the error message in a single line.
Signed-off-by: Manivannan Sadhasivam
---
drivers/virtio/virtio_pci_modern.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index e34ed4870af4..1bb55a3167a5 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -274,8 +274,8 @@ static int vp_finalize_features(struct virtio_device *vdev)
vp_transport_features(vdev, features);
if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
- dev_err(&vdev->dev, "device uses modern interface "
- "but does not have VIRTIO_F_VERSION_1\n");
+ dev_err(&vdev->dev,
+ "device uses modern interface but does not have
VIRTIO_F_VERSION_1\n");
return -EINVAL;
}
--
2.25.1
[PATCH 1/2] virtio: Remove redundant 'virtio:' prefix in error messages
dev_err() already prefixes the virtio device name which is enough for users
to know that the error is coming from the virtio driver. Adding one more
'virtio:' prefix is redundant. It results in error logs as below:
virtio_net virtio0: virtio: device uses modern interface but does not have
VIRTIO_F_VERSION_1
So remove the 'virtio:' prefix which is redundant.
Signed-off-by: Manivannan Sadhasivam
---
drivers/virtio/virtio.c| 3 +--
drivers/virtio/virtio_pci_modern.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index b968b2aa5f4d..0e06aae8071b 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -194,8 +194,7 @@ static int virtio_features_ok(struct virtio_device *dev)
virtio_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
status = dev->config->get_status(dev);
if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
- dev_err(&dev->dev, "virtio: device refuses features: %x\n",
- status);
+ dev_err(&dev->dev, "device refuses features: %x\n", status);
return -ENODEV;
}
return 0;
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index f62b530aa3b5..e34ed4870af4 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -237,7 +237,7 @@ static int __vp_check_common_size_one_feature(struct
virtio_device *vdev, u32 fb
return 0;
dev_err(&vdev->dev,
- "virtio: common cfg size(%zu) does not match the feature %s\n",
+ "common cfg size(%zu) does not match the feature %s\n",
vp_dev->mdev.common_len, fname);
return -EINVAL;
@@ -274,7 +274,7 @@ static int vp_finalize_features(struct virtio_device *vdev)
vp_transport_features(vdev, features);
if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
- dev_err(&vdev->dev, "virtio: device uses modern interface "
+ dev_err(&vdev->dev, "device uses modern interface "
"but does not have VIRTIO_F_VERSION_1\n");
return -EINVAL;
}
--
2.25.1
Re: [PATCH] virtio-pci: Add MSI support
+ Jason Wang (email got truncated).
On Fri, Jul 12, 2024 at 07:59:14PM +0530, Manivannan Sadhasivam wrote:
> Virtio spec has so far only supported MSI-X and INTX for receiving the
> interrupts from the virtio device on PCI transport. But this becomes a
> limiting factor for devices supporting only MSI (plus INTX emulation) as
> they have to use the legacy INTX emulation which is limited to one IRQ per
> PCIe function.
>
> But this now addressed with the help of a proposal to the Virtio spec
> adding MSI support [1]. Based on that, let's implement MSI support in the
> virtio-pci driver.
>
> The Virtio spec proposal reuses the existing MSI-X infrastructure, like the
> config_msix_vector/queue_msix_vector fields of the Virito common config
> structure. Following that, MSI support in virtio-pci driver is also added
> on top of the existing MSI-X implementation and it mostly reuses the MSI-X
> code base. The existing vp_find_vqs_msix() API is modified to support MSI
> along with MSI-X.
>
> The preference for interrupt allocation is still given to MSI-X as per the
> spec. The driver will try to allocate MSI only if both of the MSI-X
> allocations (one vector for each queue and 2 vectors) fails. As like MSI-X,
> driver will try to allocate one MSI vector for each queue first, and if
> that fails, it will try to allocate 2 vectors (one for config queue and one
> shared for queues). If both of them fails, driver will fallback to the
> legacy INTX as usual.
>
> For keeping the changes minimal, existing 'virtio_pci_device::msix_enabled'
> flag is used to indicate the status of MSI and MSI-X. Rest of the MSI-X
> functionalities such as IRQ affinity are also reused for MSI (but the
> affinity setting really depends on the underlying IRQCHIP controller).
>
> [1]
> https://lore.kernel.org/virtio-comment/20240712140144.12066-1-manivannan.sadhasi...@linaro.org/
>
> Signed-off-by: Manivannan Sadhasivam
> ---
> drivers/virtio/virtio_pci_common.c | 24 ++--
> drivers/virtio/virtio_pci_common.h | 2 +-
> 2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c
> b/drivers/virtio/virtio_pci_common.c
> index f6b0b00e4599..6f80b0c46c5f 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -100,11 +100,11 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
> }
>
> static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
> -bool per_vq_vectors, struct irq_affinity
> *desc)
> +bool per_vq_vectors, struct irq_affinity
> *desc,
> +unsigned int flags)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> const char *name = dev_name(&vp_dev->vdev.dev);
> - unsigned int flags = PCI_IRQ_MSIX;
> unsigned int i, v;
> int err = -ENOMEM;
>
> @@ -288,7 +288,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
> unsigned int nvqs,
> struct virtqueue *vqs[], vq_callback_t *callbacks[],
> const char * const names[], bool per_vq_vectors,
> const bool *ctx,
> - struct irq_affinity *desc)
> + struct irq_affinity *desc, unsigned int flags)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> u16 msix_vec;
> @@ -310,7 +310,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
> unsigned int nvqs,
> }
>
> err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors,
> - per_vq_vectors ? desc : NULL);
> + per_vq_vectors ? desc : NULL, flags);
> if (err)
> goto error_find;
>
> @@ -407,11 +407,23 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned
> int nvqs,
> int err;
>
> /* Try MSI-X with one vector per queue. */
> - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
> desc);
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
> +desc, PCI_IRQ_MSIX);
> if (!err)
> return 0;
> /* Fallback: MSI-X with one vector for config, one shared for queues. */
> - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
> desc);
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
> +desc, PCI_IRQ_MSIX);
> + if (!err)
> + return 0;
> + /* Try MSI with one vector per queue. */
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ct
[PATCH] virtio-pci: Add MSI support
Virtio spec has so far only supported MSI-X and INTX for receiving the
interrupts from the virtio device on PCI transport. But this becomes a
limiting factor for devices supporting only MSI (plus INTX emulation) as
they have to use the legacy INTX emulation which is limited to one IRQ per
PCIe function.
But this now addressed with the help of a proposal to the Virtio spec
adding MSI support [1]. Based on that, let's implement MSI support in the
virtio-pci driver.
The Virtio spec proposal reuses the existing MSI-X infrastructure, like the
config_msix_vector/queue_msix_vector fields of the Virito common config
structure. Following that, MSI support in virtio-pci driver is also added
on top of the existing MSI-X implementation and it mostly reuses the MSI-X
code base. The existing vp_find_vqs_msix() API is modified to support MSI
along with MSI-X.
The preference for interrupt allocation is still given to MSI-X as per the
spec. The driver will try to allocate MSI only if both of the MSI-X
allocations (one vector for each queue and 2 vectors) fails. As like MSI-X,
driver will try to allocate one MSI vector for each queue first, and if
that fails, it will try to allocate 2 vectors (one for config queue and one
shared for queues). If both of them fails, driver will fallback to the
legacy INTX as usual.
For keeping the changes minimal, existing 'virtio_pci_device::msix_enabled'
flag is used to indicate the status of MSI and MSI-X. Rest of the MSI-X
functionalities such as IRQ affinity are also reused for MSI (but the
affinity setting really depends on the underlying IRQCHIP controller).
[1]
https://lore.kernel.org/virtio-comment/20240712140144.12066-1-manivannan.sadhasi...@linaro.org/
Signed-off-by: Manivannan Sadhasivam
---
drivers/virtio/virtio_pci_common.c | 24 ++--
drivers/virtio/virtio_pci_common.h | 2 +-
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c
b/drivers/virtio/virtio_pci_common.c
index f6b0b00e4599..6f80b0c46c5f 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -100,11 +100,11 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
}
static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
- bool per_vq_vectors, struct irq_affinity
*desc)
+ bool per_vq_vectors, struct irq_affinity
*desc,
+ unsigned int flags)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
const char *name = dev_name(&vp_dev->vdev.dev);
- unsigned int flags = PCI_IRQ_MSIX;
unsigned int i, v;
int err = -ENOMEM;
@@ -288,7 +288,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
unsigned int nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
const char * const names[], bool per_vq_vectors,
const bool *ctx,
- struct irq_affinity *desc)
+ struct irq_affinity *desc, unsigned int flags)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
u16 msix_vec;
@@ -310,7 +310,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
unsigned int nvqs,
}
err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors,
- per_vq_vectors ? desc : NULL);
+ per_vq_vectors ? desc : NULL, flags);
if (err)
goto error_find;
@@ -407,11 +407,23 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned int
nvqs,
int err;
/* Try MSI-X with one vector per queue. */
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
desc);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
+ desc, PCI_IRQ_MSIX);
if (!err)
return 0;
/* Fallback: MSI-X with one vector for config, one shared for queues. */
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
desc);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
+ desc, PCI_IRQ_MSIX);
+ if (!err)
+ return 0;
+ /* Try MSI with one vector per queue. */
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
+ desc, PCI_IRQ_MSI);
+ if (!err)
+ return 0;
+ /* Fallback: MSI with one vector for config, one shared for queues. */
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
+ desc, PCI_IRQ_MSI);
if (!err)
return 0;
/* Is there an interrupt? If not give up. */
diff --git a/drivers/virtio/virtio_pci_common.h
b/drivers/virtio/virtio_pci_common.h
index 7fef52bee455..a5062ca85f3b 10064
Re: [PATCH v5 2/3] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
On Fri, Mar 22, 2024 at 08:59:12AM +0100, Luca Weiss wrote:
> On Mon Dec 4, 2023 at 6:28 PM CET, Manivannan Sadhasivam wrote:
> > On Mon, Dec 04, 2023 at 01:21:42PM +0100, Luca Weiss wrote:
> > > On Mon Dec 4, 2023 at 1:15 PM CET, Nitin Rawat wrote:
> > > >
> > > >
> > > > On 12/4/2023 3:54 PM, Luca Weiss wrote:
> > > > > From: Nitin Rawat
> > > > >
> > > > > Add UFS host controller and PHY nodes for sc7280 soc.
> > > > >
> > > > > Signed-off-by: Nitin Rawat
> > > > > Reviewed-by: Konrad Dybcio
> > > > > Tested-by: Konrad Dybcio # QCM6490 FP5
> > > > > [luca: various cleanups and additions as written in the cover letter]
> > > > > Signed-off-by: Luca Weiss
> > > > > ---
> > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 74
> > > > > +++-
> > > > > 1 file changed, 73 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > index 04bf85b0399a..8b08569f2191 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > @@ -15,6 +15,7 @@
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > +#include
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > @@ -906,7 +907,7 @@ gcc: clock-controller@10 {
> > > > > clocks = <&rpmhcc RPMH_CXO_CLK>,
> > > > ><&rpmhcc RPMH_CXO_CLK_A>, <&sleep_clk>,
> > > > ><0>, <&pcie1_phy>,
> > > > > - <0>, <0>, <0>,
> > > > > + <&ufs_mem_phy 0>, <&ufs_mem_phy 1>,
> > > > > <&ufs_mem_phy 2>,
> > > > ><&usb_1_qmpphy
> > > > > QMP_USB43DP_USB3_PIPE_CLK>;
> > > > > clock-names = "bi_tcxo", "bi_tcxo_ao",
> > > > > "sleep_clk",
> > > > > "pcie_0_pipe_clk",
> > > > > "pcie_1_pipe_clk",
> > > > > @@ -2238,6 +2239,77 @@ pcie1_phy: phy@1c0e000 {
> > > > > status = "disabled";
> > > > > };
> > > > >
> > > > > + ufs_mem_hc: ufs@1d84000 {
> > > > > + compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
> > > > > + "jedec,ufs-2.0";
> > > > > + reg = <0x0 0x01d84000 0x0 0x3000>;
> > > > > + interrupts = ;
> > > > > + phys = <&ufs_mem_phy>;
> > > > > + phy-names = "ufsphy";
> > > > > + lanes-per-direction = <2>;
> > > > > + #reset-cells = <1>;
> > > > > + resets = <&gcc GCC_UFS_PHY_BCR>;
> > > > > + reset-names = "rst";
> > > > > +
> > > > > + power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> > > > > + required-opps = <&rpmhpd_opp_nom>;
> > > > > +
> > > > > + iommus = <&apps_smmu 0x80 0x0>;
> > > > > + dma-coherent;
> > > > > +
> > > > > + interconnects = <&aggre1_noc MASTER_UFS_MEM
> > > > > QCOM_ICC_TAG_ALWAYS
> > > > > + &mc_virt SLAVE_EBI1
> > > > > QCOM_ICC_TAG_ALWAYS>,
> > > > > + <&gem_noc MASTER_APPSS_PROC
> > > > > QCOM_ICC_TAG_ALWAYS
> > > > > + &cnoc2 SLAVE_UFS_MEM_CFG
> > > > > QCOM_ICC_TAG_ALWAYS>;
> > > > >
Re: [PATCH] bus: mhi: host: Change the trace string for the userspace tools mapping
On Sun, Feb 18, 2024 at 02:13:39PM +0530, Krishna chaitanya chundru wrote:
> User space tools can't map strings if we use directly, as the string
> address is internal to kernel.
>
> So add trace point strings for the user space tools to map strings
> properly.
>
> Signed-off-by: Krishna chaitanya chundru
Applied to mhi-next!
- Mani
> ---
> drivers/bus/mhi/host/main.c | 4 ++--
> drivers/bus/mhi/host/trace.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index 2d38f6005da6..15d657af9b5b 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -1340,7 +1340,7 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> enum mhi_cmd_type cmd = MHI_CMD_NOP;
> int ret;
>
> - trace_mhi_channel_command_start(mhi_cntrl, mhi_chan, to_state,
> "Updating");
> + trace_mhi_channel_command_start(mhi_cntrl, mhi_chan, to_state,
> TPS("Updating"));
> switch (to_state) {
> case MHI_CH_STATE_TYPE_RESET:
> write_lock_irq(&mhi_chan->lock);
> @@ -1407,7 +1407,7 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> write_unlock_irq(&mhi_chan->lock);
> }
>
> - trace_mhi_channel_command_end(mhi_cntrl, mhi_chan, to_state, "Updated");
> + trace_mhi_channel_command_end(mhi_cntrl, mhi_chan, to_state,
> TPS("Updated"));
> exit_channel_update:
> mhi_cntrl->runtime_put(mhi_cntrl);
> mhi_device_put(mhi_cntrl->mhi_dev);
> diff --git a/drivers/bus/mhi/host/trace.h b/drivers/bus/mhi/host/trace.h
> index d12a98d44272..368515dcb22d 100644
> --- a/drivers/bus/mhi/host/trace.h
> +++ b/drivers/bus/mhi/host/trace.h
> @@ -84,6 +84,8 @@ DEV_ST_TRANSITION_LIST
> #define dev_st_trans(a, b) { DEV_ST_TRANSITION_##a, b },
> #define dev_st_trans_end(a, b) { DEV_ST_TRANSITION_##a, b }
>
> +#define TPS(x) tracepoint_string(x)
> +
> TRACE_EVENT(mhi_gen_tre,
>
> TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
>
> ---
> base-commit: ceeb64f41fe6a1eb9fc56d583983a81f8f3dd058
> change-id: 20240218-ftrace_string-7677762aa63c
>
> Best regards,
> --
> Krishna chaitanya chundru
>
--
மணிவண்ணன் சதாசிவம்
Re: [PATCH] bus: mhi: host: Change the trace string for the userspace tools mapping
On Wed, Feb 21, 2024 at 09:11:03AM -0500, Steven Rostedt wrote: > On Wed, 21 Feb 2024 11:41:46 +0530 > Manivannan Sadhasivam wrote: > > > On Sun, Feb 18, 2024 at 02:13:39PM +0530, Krishna chaitanya chundru wrote: > > > User space tools can't map strings if we use directly, as the string > > > address is internal to kernel. > > > > > > So add trace point strings for the user space tools to map strings > > > properly. > > > > > > Signed-off-by: Krishna chaitanya chundru > > > > Reported-by: Steven Rostedt > > Suggested-by: may be more accurate? > Sure. Will change it while applying. - Mani > -- Steve > > > Reviewed-by: Manivannan Sadhasivam > -- மணிவண்ணன் சதாசிவம்
Re: [PATCH] bus: mhi: host: Change the trace string for the userspace tools mapping
On Sun, Feb 18, 2024 at 02:13:39PM +0530, Krishna chaitanya chundru wrote:
> User space tools can't map strings if we use directly, as the string
> address is internal to kernel.
>
> So add trace point strings for the user space tools to map strings
> properly.
>
> Signed-off-by: Krishna chaitanya chundru
Reported-by: Steven Rostedt
Reviewed-by: Manivannan Sadhasivam
- Mani
> ---
> drivers/bus/mhi/host/main.c | 4 ++--
> drivers/bus/mhi/host/trace.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index 2d38f6005da6..15d657af9b5b 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -1340,7 +1340,7 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> enum mhi_cmd_type cmd = MHI_CMD_NOP;
> int ret;
>
> - trace_mhi_channel_command_start(mhi_cntrl, mhi_chan, to_state,
> "Updating");
> + trace_mhi_channel_command_start(mhi_cntrl, mhi_chan, to_state,
> TPS("Updating"));
> switch (to_state) {
> case MHI_CH_STATE_TYPE_RESET:
> write_lock_irq(&mhi_chan->lock);
> @@ -1407,7 +1407,7 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> write_unlock_irq(&mhi_chan->lock);
> }
>
> - trace_mhi_channel_command_end(mhi_cntrl, mhi_chan, to_state, "Updated");
> + trace_mhi_channel_command_end(mhi_cntrl, mhi_chan, to_state,
> TPS("Updated"));
> exit_channel_update:
> mhi_cntrl->runtime_put(mhi_cntrl);
> mhi_device_put(mhi_cntrl->mhi_dev);
> diff --git a/drivers/bus/mhi/host/trace.h b/drivers/bus/mhi/host/trace.h
> index d12a98d44272..368515dcb22d 100644
> --- a/drivers/bus/mhi/host/trace.h
> +++ b/drivers/bus/mhi/host/trace.h
> @@ -84,6 +84,8 @@ DEV_ST_TRANSITION_LIST
> #define dev_st_trans(a, b) { DEV_ST_TRANSITION_##a, b },
> #define dev_st_trans_end(a, b) { DEV_ST_TRANSITION_##a, b }
>
> +#define TPS(x) tracepoint_string(x)
> +
> TRACE_EVENT(mhi_gen_tre,
>
> TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
>
> ---
> base-commit: ceeb64f41fe6a1eb9fc56d583983a81f8f3dd058
> change-id: 20240218-ftrace_string-7677762aa63c
>
> Best regards,
> --
> Krishna chaitanya chundru
>
--
மணிவண்ணன் சதாசிவம்
Re: [PATCH v11] bus: mhi: host: Add tracing support
On Tue, Feb 06, 2024 at 10:02:05AM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Change the implementation of the arrays which has enum to strings mapping
> to make it consistent in both trace header file and other files.
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru
Applied to mhi-next!
- Mani
> Reviewed-by: Manivannan Sadhasivam
> Reviewed-by: "Steven Rostedt (Google)"
> ---
> Changes in v11:
> - Rebased with mhi next.
> - Link to v10:
> https://lore.kernel.org/r/20240131-ftrace_support-v10-1-4349306b8...@quicinc.com
>
> Changes in v10:
> - Modified command_start and command_end traces to take string as input to
> mention correct
> - string as suggested by mani
> - As sugggested by mani modified the print format from lower format to upper
> case format.
> - Link to v9:
> https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
>
> Changes in v9:
> - Change the implementations of some array so that the strings to enum mapping
> - is same in both trace header and other files as suggested by steve.
> - Link to v8:
> https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
>
> Changes in v8:
> - Pass the structure and derefernce the variables in TP_fast_assign as
> suggested by steve
> - Link to v7:
> https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
>
> Changes in v7:
> - change log format as pointed by mani.
> - Link to v6:
> https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
>
> Changes in v6:
> - use 'rp' directly as suggested by jeffrey.
> - Link to v5:
> https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
>
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address
> to avoid
> - warnings in some platforms.
> - Link to v4:
> https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation
> issue is not
> - seen in my workspace.
> - Link to v3:
> https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/
> so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in
> the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2:
> https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as
> suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1:
> https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
> ---
> drivers/bus/mhi/common.h| 38 +++---
> drivers/bus/mhi/host/init.c | 64 +
> drivers/bus/mhi/host/internal.h | 41 ++
> drivers/bus/mhi/host/main.c | 19 ++-
> drivers/bus/mhi/host/pm.c | 7 +-
> drivers/bus/mhi/host/trace.h| 280
>
> 6 files changed, 384 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
> index f794b9c8049e..dda340aaed95 100644
> --- a/drivers/bus/mhi/common.h
> +++ b/drivers/bus/mhi/common.h
> @@ -297,30 +297,30 @@ struct mhi_ring_element {
> __le32 dword[2];
> };
>
> +#define MHI_STATE_LIST \
> + mhi_state(RESET,"RESET")\
> + mhi_state(READY,"READY")\
> + mhi_state(M0, "M0") \
> + mhi_state(M1, "M1") \
> +
Re: [PATCH v11] bus: mhi: host: Add tracing support
On Tue, Feb 06, 2024 at 02:33:12PM +0530, Manivannan Sadhasivam wrote: > On Tue, Feb 06, 2024 at 12:25:30PM +0530, Krishna Chaitanya Chundru wrote: > > > > > > On 2/6/2024 11:56 AM, Manivannan Sadhasivam wrote: > > > On Tue, Feb 06, 2024 at 10:02:05AM +0530, Krishna chaitanya chundru wrote: > > > > This change adds ftrace support for following functions which > > > > helps in debugging the issues when there is Channel state & MHI > > > > state change and also when we receive data and control events: > > > > 1. mhi_intvec_mhi_states > > > > 2. mhi_process_data_event_ring > > > > 3. mhi_process_ctrl_ev_ring > > > > 4. mhi_gen_tre > > > > 5. mhi_update_channel_state > > > > 6. mhi_tryset_pm_state > > > > 7. mhi_pm_st_worker > > > > > > > > Change the implementation of the arrays which has enum to strings > > > > mapping > > > > to make it consistent in both trace header file and other files. > > > > > > > > Where ever the trace events are added, debug messages are removed. > > > > > > > > Signed-off-by: Krishna chaitanya chundru > > > > > > There are a lot of checkpatch errors. Please fix them and resubmit. > > > > > > - Mani > > > > > Hi Mani, > > > > The errors which is pointing in the checkpatch are false positive, those > > errors are being shown from v2 onwards and kernel test boot is also not > > throwing any errors for it. > > > > Bot will check sparse warnings/errors mostly. But these checkpatch issues can > be > fixed easily. If you don't do it now, then someone will send a patch for it > later. > Hmm, seems like we should ignore these checkpatch issues due to the way the macros are used by trace headers. Ignore my above comment, patch looks fine. - Mani > - Mani > > > I checked with internal team they said these errors are false positive. > > > > Thanks & Regards, > > Krishna Chaitanya. > > > > > > Reviewed-by: Manivannan Sadhasivam > > > > Reviewed-by: "Steven Rostedt (Google)" > > > > --- > > > > Changes in v11: > > > > - Rebased with mhi next. > > > > - Link to v10: > > > > https://lore.kernel.org/r/20240131-ftrace_support-v10-1-4349306b8...@quicinc.com > > > > > > > > Changes in v10: > > > > - Modified command_start and command_end traces to take string as input > > > > to mention correct > > > > - string as suggested by mani > > > > - As sugggested by mani modified the print format from lower format to > > > > upper case format. > > > > - Link to v9: > > > > https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com > > > > > > > > Changes in v9: > > > > - Change the implementations of some array so that the strings to enum > > > > mapping > > > > - is same in both trace header and other files as suggested by steve. > > > > - Link to v8: > > > > https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com > > > > > > > > Changes in v8: > > > > - Pass the structure and derefernce the variables in TP_fast_assign as > > > > suggested by steve > > > > - Link to v7: > > > > https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com > > > > > > > > Changes in v7: > > > > - change log format as pointed by mani. > > > > - Link to v6: > > > > https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com > > > > > > > > Changes in v6: > > > > - use 'rp' directly as suggested by jeffrey. > > > > - Link to v5: > > > > https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com > > > > > > > > Changes in v5: > > > > - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve. > > > > - Instead of converting to u64 to print address, use %px to print the > > > > address to avoid > > > > - warnings in some platforms. > > > > - Link to v4: > > > > https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com > > > > > > > > Changes in v4: > > > > - Fix compilation issues in previous patch which happended due to > > > > rebasing. > >
Re: [PATCH v11] bus: mhi: host: Add tracing support
On Tue, Feb 06, 2024 at 12:25:30PM +0530, Krishna Chaitanya Chundru wrote: > > > On 2/6/2024 11:56 AM, Manivannan Sadhasivam wrote: > > On Tue, Feb 06, 2024 at 10:02:05AM +0530, Krishna chaitanya chundru wrote: > > > This change adds ftrace support for following functions which > > > helps in debugging the issues when there is Channel state & MHI > > > state change and also when we receive data and control events: > > > 1. mhi_intvec_mhi_states > > > 2. mhi_process_data_event_ring > > > 3. mhi_process_ctrl_ev_ring > > > 4. mhi_gen_tre > > > 5. mhi_update_channel_state > > > 6. mhi_tryset_pm_state > > > 7. mhi_pm_st_worker > > > > > > Change the implementation of the arrays which has enum to strings mapping > > > to make it consistent in both trace header file and other files. > > > > > > Where ever the trace events are added, debug messages are removed. > > > > > > Signed-off-by: Krishna chaitanya chundru > > > > There are a lot of checkpatch errors. Please fix them and resubmit. > > > > - Mani > > > Hi Mani, > > The errors which is pointing in the checkpatch are false positive, those > errors are being shown from v2 onwards and kernel test boot is also not > throwing any errors for it. > Bot will check sparse warnings/errors mostly. But these checkpatch issues can be fixed easily. If you don't do it now, then someone will send a patch for it later. - Mani > I checked with internal team they said these errors are false positive. > > Thanks & Regards, > Krishna Chaitanya. > > > > Reviewed-by: Manivannan Sadhasivam > > > Reviewed-by: "Steven Rostedt (Google)" > > > --- > > > Changes in v11: > > > - Rebased with mhi next. > > > - Link to v10: > > > https://lore.kernel.org/r/20240131-ftrace_support-v10-1-4349306b8...@quicinc.com > > > > > > Changes in v10: > > > - Modified command_start and command_end traces to take string as input > > > to mention correct > > > - string as suggested by mani > > > - As sugggested by mani modified the print format from lower format to > > > upper case format. > > > - Link to v9: > > > https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com > > > > > > Changes in v9: > > > - Change the implementations of some array so that the strings to enum > > > mapping > > > - is same in both trace header and other files as suggested by steve. > > > - Link to v8: > > > https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com > > > > > > Changes in v8: > > > - Pass the structure and derefernce the variables in TP_fast_assign as > > > suggested by steve > > > - Link to v7: > > > https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com > > > > > > Changes in v7: > > > - change log format as pointed by mani. > > > - Link to v6: > > > https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com > > > > > > Changes in v6: > > > - use 'rp' directly as suggested by jeffrey. > > > - Link to v5: > > > https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com > > > > > > Changes in v5: > > > - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve. > > > - Instead of converting to u64 to print address, use %px to print the > > > address to avoid > > > - warnings in some platforms. > > > - Link to v4: > > > https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com > > > > > > Changes in v4: > > > - Fix compilation issues in previous patch which happended due to > > > rebasing. > > > - In the defconfig FTRACE config is not enabled due to that the > > > compilation issue is not > > > - seen in my workspace. > > > - Link to v3: > > > https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com > > > > > > Changes in v3: > > > - move trace header file from include/trace/events to > > > drivers/bus/mhi/host/ so that > > > - we can include driver header files. > > > - Use macros directly in the trace events as suggested Jeffrey Hugo. > > > - Reorder the structure in the events as suggested by steve to avoid > > > holes in the buffer. > > > - removed the mhi_to_physical
Re: [PATCH v11] bus: mhi: host: Add tracing support
On Tue, Feb 06, 2024 at 10:02:05AM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Change the implementation of the arrays which has enum to strings mapping
> to make it consistent in both trace header file and other files.
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru
There are a lot of checkpatch errors. Please fix them and resubmit.
- Mani
> Reviewed-by: Manivannan Sadhasivam
> Reviewed-by: "Steven Rostedt (Google)"
> ---
> Changes in v11:
> - Rebased with mhi next.
> - Link to v10:
> https://lore.kernel.org/r/20240131-ftrace_support-v10-1-4349306b8...@quicinc.com
>
> Changes in v10:
> - Modified command_start and command_end traces to take string as input to
> mention correct
> - string as suggested by mani
> - As sugggested by mani modified the print format from lower format to upper
> case format.
> - Link to v9:
> https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
>
> Changes in v9:
> - Change the implementations of some array so that the strings to enum mapping
> - is same in both trace header and other files as suggested by steve.
> - Link to v8:
> https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
>
> Changes in v8:
> - Pass the structure and derefernce the variables in TP_fast_assign as
> suggested by steve
> - Link to v7:
> https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
>
> Changes in v7:
> - change log format as pointed by mani.
> - Link to v6:
> https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
>
> Changes in v6:
> - use 'rp' directly as suggested by jeffrey.
> - Link to v5:
> https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
>
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address
> to avoid
> - warnings in some platforms.
> - Link to v4:
> https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation
> issue is not
> - seen in my workspace.
> - Link to v3:
> https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/
> so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in
> the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2:
> https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as
> suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1:
> https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
> ---
> drivers/bus/mhi/common.h| 38 +++---
> drivers/bus/mhi/host/init.c | 64 +
> drivers/bus/mhi/host/internal.h | 41 ++
> drivers/bus/mhi/host/main.c | 19 ++-
> drivers/bus/mhi/host/pm.c | 7 +-
> drivers/bus/mhi/host/trace.h| 280
>
> 6 files changed, 384 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
> index f794b9c8049e..dda340aaed95 100644
> --- a/drivers/bus/mhi/common.h
> +++ b/drivers/bus/mhi/common.h
> @@ -297,30 +297,30 @@ struct mhi_ring_element {
> __le32 dword[2];
> };
>
> +#define MHI_STATE_LIST \
> + mhi_state(RESET,"RESET")\
> + mhi_state(READY,"READY")\
> + mhi_state(M0, "M0") \
> + mhi_state(M
Re: [PATCH v10] bus: mhi: host: Add tracing support
On Wed, Jan 31, 2024 at 09:54:04AM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Change the implementation of the arrays which has enum to strings mapping
> to make it consistent in both trace header file and other files.
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru
This fails to apply cleanly. Please rebase on top of mhi-next and resend.
- Mani
> Reviewed-by: "Steven Rostedt (Google)"
> ---
> Changes in v10:
> - Modified command_start and command_end traces to take string as input to
> mention correct
> - string as suggested by mani
> - As sugggested by mani modified the print format from lower format to upper
> case format.
> - Link to v9:
> https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
>
> Changes in v9:
> - Change the implementations of some array so that the strings to enum mapping
> - is same in both trace header and other files as suggested by steve.
> - Link to v8:
> https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
>
> Changes in v8:
> - Pass the structure and derefernce the variables in TP_fast_assign as
> suggested by steve
> - Link to v7:
> https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
>
> Changes in v7:
> - change log format as pointed by mani.
> - Link to v6:
> https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
>
> Changes in v6:
> - use 'rp' directly as suggested by jeffrey.
> - Link to v5:
> https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
>
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address
> to avoid
> - warnings in some platforms.
> - Link to v4:
> https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation
> issue is not
> - seen in my workspace.
> - Link to v3:
> https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/
> so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in
> the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2:
> https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as
> suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1:
> https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
> ---
> drivers/bus/mhi/common.h| 38 +++---
> drivers/bus/mhi/host/init.c | 63 +
> drivers/bus/mhi/host/internal.h | 40 ++
> drivers/bus/mhi/host/main.c | 19 ++-
> drivers/bus/mhi/host/pm.c | 7 +-
> drivers/bus/mhi/host/trace.h| 280
>
> 6 files changed, 383 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
> index f794b9c8049e..dda340aaed95 100644
> --- a/drivers/bus/mhi/common.h
> +++ b/drivers/bus/mhi/common.h
> @@ -297,30 +297,30 @@ struct mhi_ring_element {
> __le32 dword[2];
> };
>
> +#define MHI_STATE_LIST \
> + mhi_state(RESET,"RESET")\
> + mhi_state(READY,"READY")\
> + mhi_state(M0, "M0") \
> + mhi_state(M1, "M1") \
> + mhi_state(M2, "M2") \
> + mhi_state(M3, "M3") \
> + mhi_state(M3_FAST, "M3_FAST") \
> + mhi_state(BHI, "BHI") \
> + mhi_state_end(SYS_ERR, "SYS ERROR")
> +
> +#undef mhi_state
> +#undef mhi_state_end
> +
> +#define mhi_state(a, b) case MHI_STATE_##a: return b;
> +#define mhi_state_end(a, b) case MHI_STATE_##a: return b;
> +
> static inline const char *mhi_state_str(enum mhi_state state)
> {
> switch (state) {
> - case MHI_STATE_RESET:
Re: [PATCH v10] bus: mhi: host: Add tracing support
On Wed, Jan 31, 2024 at 09:54:04AM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Change the implementation of the arrays which has enum to strings mapping
> to make it consistent in both trace header file and other files.
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru
Reviewed-by: Manivannan Sadhasivam
- Mani
> Reviewed-by: "Steven Rostedt (Google)"
> ---
> Changes in v10:
> - Modified command_start and command_end traces to take string as input to
> mention correct
> - string as suggested by mani
> - As sugggested by mani modified the print format from lower format to upper
> case format.
> - Link to v9:
> https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
>
> Changes in v9:
> - Change the implementations of some array so that the strings to enum mapping
> - is same in both trace header and other files as suggested by steve.
> - Link to v8:
> https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
>
> Changes in v8:
> - Pass the structure and derefernce the variables in TP_fast_assign as
> suggested by steve
> - Link to v7:
> https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
>
> Changes in v7:
> - change log format as pointed by mani.
> - Link to v6:
> https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
>
> Changes in v6:
> - use 'rp' directly as suggested by jeffrey.
> - Link to v5:
> https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
>
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address
> to avoid
> - warnings in some platforms.
> - Link to v4:
> https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation
> issue is not
> - seen in my workspace.
> - Link to v3:
> https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/
> so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in
> the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2:
> https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as
> suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1:
> https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
> ---
> drivers/bus/mhi/common.h| 38 +++---
> drivers/bus/mhi/host/init.c | 63 +
> drivers/bus/mhi/host/internal.h | 40 ++
> drivers/bus/mhi/host/main.c | 19 ++-
> drivers/bus/mhi/host/pm.c | 7 +-
> drivers/bus/mhi/host/trace.h| 280
>
> 6 files changed, 383 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
> index f794b9c8049e..dda340aaed95 100644
> --- a/drivers/bus/mhi/common.h
> +++ b/drivers/bus/mhi/common.h
> @@ -297,30 +297,30 @@ struct mhi_ring_element {
> __le32 dword[2];
> };
>
> +#define MHI_STATE_LIST \
> + mhi_state(RESET,"RESET")\
> + mhi_state(READY,"READY")\
> + mhi_state(M0, "M0") \
> + mhi_state(M1, "M1") \
> + mhi_state(M2, "M2") \
> + mhi_state(M3, "M3") \
> + mhi_state(M3_FAST, "M3_FAST") \
> +
Re: [PATCH v9] bus: mhi: host: Add tracing support
On Tue, Jan 30, 2024 at 09:22:52AM -0500, Steven Rostedt wrote: > On Tue, 30 Jan 2024 13:41:52 +0530 > Manivannan Sadhasivam wrote: > > > So same trace will get printed for both mhi_channel_command_start() and > > mhi_channel_command_end()? > > The trace output will also include the tracepoint name. That is, it will > have the same content but will be preceded with: > > mhi_channel_command_start: ... > mhi_channel_command_end: ... > Yes, but the message will be the same: mhi_channel_command_start: chan%d: Updating state to: mhi_channel_command_end: chan%d: Updating state to: Either only one of the trace should be present or the second one should print, "mhi_channel_command_end: chan%d: Updated state to:" - Mani -- மணிவண்ணன் சதாசிவம்
Re: [PATCH v9] bus: mhi: host: Add tracing support
On Fri, Jan 05, 2024 at 05:53:03PM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Change the implementation of the arrays which has enum to strings mapping
> to make it consistent in both trace header file and other files.
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru
Few nitpicks below.
> Reviewed-by: "Steven Rostedt (Google)"
> ---
> Changes in v9:
> - Change the implementations of some array so that the strings to enum mapping
> - is same in both trace header and other files as suggested by steve.
> - Link to v8:
> https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
>
> Changes in v8:
> - Pass the structure and derefernce the variables in TP_fast_assign as
> suggested by steve
> - Link to v7:
> https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
>
> Changes in v7:
> - change log format as pointed by mani.
> - Link to v6:
> https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
>
> Changes in v6:
> - use 'rp' directly as suggested by jeffrey.
> - Link to v5:
> https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
>
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address
> to avoid
> - warnings in some platforms.
> - Link to v4:
> https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation
> issue is not
> - seen in my workspace.
> - Link to v3:
> https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/
> so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in
> the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2:
> https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as
> suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1:
> https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
> ---
> drivers/bus/mhi/common.h| 38 +++---
> drivers/bus/mhi/host/init.c | 63 +
> drivers/bus/mhi/host/internal.h | 40 ++
> drivers/bus/mhi/host/main.c | 19 ++-
> drivers/bus/mhi/host/pm.c | 7 +-
> drivers/bus/mhi/host/trace.h| 275
>
> 6 files changed, 378 insertions(+), 64 deletions(-)
>
[...]
> +TRACE_EVENT(mhi_gen_tre,
> +
> + TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
> + struct mhi_ring_element *mhi_tre),
> +
> + TP_ARGS(mhi_cntrl, mhi_chan, mhi_tre),
> +
> + TP_STRUCT__entry(
> + __string(name, mhi_cntrl->mhi_dev->name)
> + __field(int, ch_num)
> + __field(void *, wp)
> + __field(__le64, tre_ptr)
> + __field(__le32, dword0)
> + __field(__le32, dword1)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(name, mhi_cntrl->mhi_dev->name);
> + __entry->ch_num = mhi_chan->chan;
> + __entry->wp = mhi_tre;
> + __entry->tre_ptr = mhi_tre->ptr;
> + __entry->dword0 = mhi_tre->dword[0];
> + __entry->dword1 = mhi_tre->dword[1];
> + ),
> +
> + TP_printk("%s: Chan: %d Tre: 0x%p Tre buf: 0x%llx dword0: 0x%08x
> dword1: 0x%08x\n",
Use caps for printing the acronyms everywhere. Like TRE, DWORD etc...
> + __get_str(name), __entry->ch_num, __entry->wp,
> __entry->tre_ptr,
> + __entry->dword0, __entry->dword1)
> +);
> +
> +TRACE_EVENT(mhi_intvec_states,
> +
> + TP_PROTO(struct mhi_controller *mhi_cntrl, int dev_ee, int dev_state),
> +
> + TP_ARGS(mhi_cntrl, dev_ee, dev_state),
> +
> + TP_STRUCT__entry(
> + __string(name, mhi_cntrl->mhi_dev->name)
> + __field(int, local_ee)
> +
Re: [PATCH v6 2/3] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
On Tue, Dec 05, 2023 at 03:38:55PM +0100, Luca Weiss wrote:
> From: Nitin Rawat
>
> Add UFS host controller and PHY nodes for sc7280 soc.
>
> Signed-off-by: Nitin Rawat
> Reviewed-by: Konrad Dybcio
> Tested-by: Konrad Dybcio # QCM6490 FP5
> [luca: various cleanups and additions as written in the cover letter]
> Signed-off-by: Luca Weiss
Acked-by: Manivannan Sadhasivam
- Mani
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 74
> +++-
> 1 file changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 04bf85b0399a..dcb6c2004f87 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -906,7 +907,7 @@ gcc: clock-controller@10 {
> clocks = <&rpmhcc RPMH_CXO_CLK>,
><&rpmhcc RPMH_CXO_CLK_A>, <&sleep_clk>,
><0>, <&pcie1_phy>,
> - <0>, <0>, <0>,
> + <&ufs_mem_phy 0>, <&ufs_mem_phy 1>,
> <&ufs_mem_phy 2>,
><&usb_1_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
> clock-names = "bi_tcxo", "bi_tcxo_ao", "sleep_clk",
> "pcie_0_pipe_clk", "pcie_1_pipe_clk",
> @@ -2238,6 +2239,77 @@ pcie1_phy: phy@1c0e000 {
> status = "disabled";
> };
>
> + ufs_mem_hc: ufs@1d84000 {
> + compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
> + "jedec,ufs-2.0";
> + reg = <0x0 0x01d84000 0x0 0x3000>;
> + interrupts = ;
> + phys = <&ufs_mem_phy>;
> + phy-names = "ufsphy";
> + lanes-per-direction = <2>;
> + #reset-cells = <1>;
> + resets = <&gcc GCC_UFS_PHY_BCR>;
> + reset-names = "rst";
> +
> + power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> + required-opps = <&rpmhpd_opp_nom>;
> +
> + iommus = <&apps_smmu 0x80 0x0>;
> + dma-coherent;
> +
> + interconnects = <&aggre1_noc MASTER_UFS_MEM
> QCOM_ICC_TAG_ALWAYS
> + &mc_virt SLAVE_EBI1
> QCOM_ICC_TAG_ALWAYS>,
> + <&gem_noc MASTER_APPSS_PROC
> QCOM_ICC_TAG_ALWAYS
> + &cnoc2 SLAVE_UFS_MEM_CFG
> QCOM_ICC_TAG_ALWAYS>;
> + interconnect-names = "ufs-ddr", "cpu-ufs";
> +
> + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> + <&gcc GCC_UFS_PHY_AHB_CLK>,
> + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>,
> + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
> + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
> + clock-names = "core_clk",
> + "bus_aggr_clk",
> + "iface_clk",
> + "core_clk_unipro",
> + "ref_clk",
> + "tx_lane0_sync_clk",
> + "rx_lane0_sync_clk",
> + "rx_lane1_sync_clk";
> + freq-table-hz =
> + <7500 3>,
> + <0 0>,
> + <0 0>,
> + <7500 3>,
> + <0 0>,
> + <0 0>,
> + <0 0>,
> + <0 0>;
> +
Re: [PATCH v5 2/3] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
On Tue, Dec 05, 2023 at 08:51:05AM +0100, Luca Weiss wrote:
> On Mon Dec 4, 2023 at 6:28 PM CET, Manivannan Sadhasivam wrote:
> > On Mon, Dec 04, 2023 at 01:21:42PM +0100, Luca Weiss wrote:
> > > On Mon Dec 4, 2023 at 1:15 PM CET, Nitin Rawat wrote:
> > > >
> > > >
> > > > On 12/4/2023 3:54 PM, Luca Weiss wrote:
> > > > > From: Nitin Rawat
> > > > >
> > > > > Add UFS host controller and PHY nodes for sc7280 soc.
> > > > >
> > > > > Signed-off-by: Nitin Rawat
> > > > > Reviewed-by: Konrad Dybcio
> > > > > Tested-by: Konrad Dybcio # QCM6490 FP5
> > > > > [luca: various cleanups and additions as written in the cover letter]
> > > > > Signed-off-by: Luca Weiss
> > > > > ---
> > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 74
> > > > > +++-
> > > > > 1 file changed, 73 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > index 04bf85b0399a..8b08569f2191 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > > > @@ -15,6 +15,7 @@
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > +#include
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > @@ -906,7 +907,7 @@ gcc: clock-controller@10 {
> > > > > clocks = <&rpmhcc RPMH_CXO_CLK>,
> > > > ><&rpmhcc RPMH_CXO_CLK_A>, <&sleep_clk>,
> > > > ><0>, <&pcie1_phy>,
> > > > > - <0>, <0>, <0>,
> > > > > + <&ufs_mem_phy 0>, <&ufs_mem_phy 1>,
> > > > > <&ufs_mem_phy 2>,
> > > > ><&usb_1_qmpphy
> > > > > QMP_USB43DP_USB3_PIPE_CLK>;
> > > > > clock-names = "bi_tcxo", "bi_tcxo_ao",
> > > > > "sleep_clk",
> > > > > "pcie_0_pipe_clk",
> > > > > "pcie_1_pipe_clk",
> > > > > @@ -2238,6 +2239,77 @@ pcie1_phy: phy@1c0e000 {
> > > > > status = "disabled";
> > > > > };
> > > > >
> > > > > + ufs_mem_hc: ufs@1d84000 {
> > > > > + compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
> > > > > + "jedec,ufs-2.0";
> > > > > + reg = <0x0 0x01d84000 0x0 0x3000>;
> > > > > + interrupts = ;
> > > > > + phys = <&ufs_mem_phy>;
> > > > > + phy-names = "ufsphy";
> > > > > + lanes-per-direction = <2>;
> > > > > + #reset-cells = <1>;
> > > > > + resets = <&gcc GCC_UFS_PHY_BCR>;
> > > > > + reset-names = "rst";
> > > > > +
> > > > > + power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> > > > > + required-opps = <&rpmhpd_opp_nom>;
> > > > > +
> > > > > + iommus = <&apps_smmu 0x80 0x0>;
> > > > > + dma-coherent;
> > > > > +
> > > > > + interconnects = <&aggre1_noc MASTER_UFS_MEM
> > > > > QCOM_ICC_TAG_ALWAYS
> > > > > + &mc_virt SLAVE_EBI1
> > > > > QCOM_ICC_TAG_ALWAYS>,
> > > > > + <&gem_noc MASTER_APPSS_PROC
> > > > > QCOM_ICC_TAG_ALWAYS
> > > > > + &cnoc2 SLAVE_UFS_MEM_CFG
> > > > > QCOM_ICC_TAG_ALWAYS>;
> > > > >
Re: [PATCH v5 2/3] arm64: dts: qcom: sc7280: Add UFS nodes for sc7280 soc
On Mon, Dec 04, 2023 at 01:21:42PM +0100, Luca Weiss wrote:
> On Mon Dec 4, 2023 at 1:15 PM CET, Nitin Rawat wrote:
> >
> >
> > On 12/4/2023 3:54 PM, Luca Weiss wrote:
> > > From: Nitin Rawat
> > >
> > > Add UFS host controller and PHY nodes for sc7280 soc.
> > >
> > > Signed-off-by: Nitin Rawat
> > > Reviewed-by: Konrad Dybcio
> > > Tested-by: Konrad Dybcio # QCM6490 FP5
> > > [luca: various cleanups and additions as written in the cover letter]
> > > Signed-off-by: Luca Weiss
> > > ---
> > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 74
> > > +++-
> > > 1 file changed, 73 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > index 04bf85b0399a..8b08569f2191 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > @@ -15,6 +15,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -906,7 +907,7 @@ gcc: clock-controller@10 {
> > > clocks = <&rpmhcc RPMH_CXO_CLK>,
> > ><&rpmhcc RPMH_CXO_CLK_A>, <&sleep_clk>,
> > ><0>, <&pcie1_phy>,
> > > - <0>, <0>, <0>,
> > > + <&ufs_mem_phy 0>, <&ufs_mem_phy 1>,
> > > <&ufs_mem_phy 2>,
> > ><&usb_1_qmpphy
> > > QMP_USB43DP_USB3_PIPE_CLK>;
> > > clock-names = "bi_tcxo", "bi_tcxo_ao",
> > > "sleep_clk",
> > > "pcie_0_pipe_clk",
> > > "pcie_1_pipe_clk",
> > > @@ -2238,6 +2239,77 @@ pcie1_phy: phy@1c0e000 {
> > > status = "disabled";
> > > };
> > >
> > > + ufs_mem_hc: ufs@1d84000 {
> > > + compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
> > > + "jedec,ufs-2.0";
> > > + reg = <0x0 0x01d84000 0x0 0x3000>;
> > > + interrupts = ;
> > > + phys = <&ufs_mem_phy>;
> > > + phy-names = "ufsphy";
> > > + lanes-per-direction = <2>;
> > > + #reset-cells = <1>;
> > > + resets = <&gcc GCC_UFS_PHY_BCR>;
> > > + reset-names = "rst";
> > > +
> > > + power-domains = <&gcc GCC_UFS_PHY_GDSC>;
> > > + required-opps = <&rpmhpd_opp_nom>;
> > > +
> > > + iommus = <&apps_smmu 0x80 0x0>;
> > > + dma-coherent;
> > > +
> > > + interconnects = <&aggre1_noc MASTER_UFS_MEM
> > > QCOM_ICC_TAG_ALWAYS
> > > + &mc_virt SLAVE_EBI1
> > > QCOM_ICC_TAG_ALWAYS>,
> > > + <&gem_noc MASTER_APPSS_PROC
> > > QCOM_ICC_TAG_ALWAYS
> > > + &cnoc2 SLAVE_UFS_MEM_CFG
> > > QCOM_ICC_TAG_ALWAYS>;
> > > + interconnect-names = "ufs-ddr", "cpu-ufs";
> > > +
> > > + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> > > + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> > > + <&gcc GCC_UFS_PHY_AHB_CLK>,
> > > + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> > > + <&rpmhcc RPMH_CXO_CLK>,
> > > + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
> > > + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
> > > + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
> > > + clock-names = "core_clk",
> > > + "bus_aggr_clk",
> > > + "iface_clk",
> > > + "core_clk_unipro",
> > > + "ref_clk",
> > > + "tx_lane0_sync_clk",
> > > + "rx_lane0_sync_clk",
> > > + "rx_lane1_sync_clk";
> > > + freq-table-hz =
> > > + <7500 3>,
> > > + <0 0>,
> > > + <0 0>,
> > > + <7500 3>,
> > > + <0 0>,
> > > + <0 0>,
> > > + <0 0>,
> > > + <0 0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + ufs_mem_phy: phy@1d87000 {
> > > + compatible = "qcom,sc7280-qmp-ufs-phy";
> > > + reg = <0x0 0x01d87000 0x0 0xe00>;
> > > + clocks = <&rpmhcc RPMH_CXO_CLK>,
> > > + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>,
> > > + <&gcc GCC_UFS_1_CLKREF_EN>;
> > > + clock-names = "ref", "ref_aux", "qref";
> > > +
> > > + power-domains = <&gc
Re: [PATCH v5] bus: mhi: host: Add tracing support
On Mon, Nov 27, 2023 at 04:39:12PM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru
> ---
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address
> to avoid
> - warnings in some platforms.
> - Link to v4:
> https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation
> issue is not
> - seen in my workspace.
> - Link to v3:
> https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/
> so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in
> the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2:
> https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as
> suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1:
> https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
> ---
> drivers/bus/mhi/host/init.c | 3 +
> drivers/bus/mhi/host/internal.h | 1 +
> drivers/bus/mhi/host/main.c | 23 +++--
> drivers/bus/mhi/host/pm.c | 6 +-
> drivers/bus/mhi/host/trace.h| 208
>
> 5 files changed, 228 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
> index f78aefd2d7a3..6acb85f4c5f8 100644
> --- a/drivers/bus/mhi/host/init.c
> +++ b/drivers/bus/mhi/host/init.c
> @@ -20,6 +20,9 @@
> #include
> #include "internal.h"
>
> +#define CREATE_TRACE_POINTS
> +#include "trace.h"
> +
> static DEFINE_IDA(mhi_controller_ida);
>
> const char * const mhi_ee_str[MHI_EE_MAX] = {
> diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
> index 2e139e76de4c..a02a71605907 100644
> --- a/drivers/bus/mhi/host/internal.h
> +++ b/drivers/bus/mhi/host/internal.h
> @@ -7,6 +7,7 @@
> #ifndef _MHI_INT_H
> #define _MHI_INT_H
>
> +#include "trace.h"
Please include header in files where it is actually used.
> #include "../common.h"
>
> extern struct bus_type mhi_bus_type;
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index dcf627b36e82..0d7e068e713a 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
> void *priv)
>
> state = mhi_get_mhi_state(mhi_cntrl);
> ee = mhi_get_exec_env(mhi_cntrl);
> - dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
> - TO_MHI_EXEC_STR(mhi_cntrl->ee),
> - mhi_state_str(mhi_cntrl->dev_state),
> - TO_MHI_EXEC_STR(ee), mhi_state_str(state));
>
> + trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
> + mhi_cntrl->dev_state, ee, state);
> if (state == MHI_STATE_SYS_ERR) {
> dev_dbg(dev, "System error detected\n");
> pm_state = mhi_tryset_pm_state(mhi_cntrl,
> @@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
> while (dev_rp != local_rp) {
> enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
>
> + trace_mhi_ctrl_event(mhi_cntrl->mhi_dev->name, local_rp,
> + local_rp->ptr, local_rp->dword[0],
> + local_rp->dword[1]);
> +
> switch (type) {
> case MHI_PKT_TYPE_BW_REQ_EVENT:
> {
> @@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller
> *mhi_cntrl,
> while (dev_rp != local_rp && event_quota > 0) {
> enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
>
> + trace_mhi_data_event(mhi_cntrl->mhi_dev->name, local_rp,
> local_rp->ptr,
> +
Re: MHI changes for v5.13
Hi Greg, On Sun, Apr 11, 2021 at 08:57:03AM +0200, Greg KH wrote: > On Sun, Apr 11, 2021 at 11:25:59AM +0530, Manivannan Sadhasivam wrote: > > Hi Greg, > > > > Here is the MHI Pull request for the v5.13 cycle. I stayed with the PR as > > the > > number patches got increased. > > > > Details are in the signed tag, please consider merging! > > A suggestion, please put [GIT PULL] in your subject line, so that I know > what this is for, that's a semi-standard thing these days. > Sorry, sent it a hurry so missed it. > Also, you have a number of patches in this set that look like they > should be backported to stable kernels. Any reason why you did not tag > them with a "Cc: stable@..." tag in the commit? After they are merged > into Linus's tree, please send the git ids of the commits that should go > to stable kernels to sta...@vger.kernel.org so that we can add them > properly. > Should've done that. Will send them once merged into mainline. Thanks, Mani > Now pulled and pushed out, thanks. > > thanks, > > greg k-h
MHI changes for v5.13
WQ_MEM_RECLAIM flag from state workqueue bus: mhi: core: Fix invalid error returning in mhi_queue bus: mhi: core: Rename debugfs directory name bus: mhi: Early MHI resume failure in non M3 state bus: mhi: core: Fix MHI runtime_pm behavior bus: mhi: pm: reduce PM state change verbosity bus: mhi: pci_generic: Implement PCI shutdown callback bus: mhi: pci_generic: Add FIREHOSE channels Manivannan Sadhasivam (2): bus: mhi: pci_generic: Constify mhi_controller_config struct definitions bus: mhi: core: Fix shadow declarations drivers/bus/mhi/core/boot.c | 64 ++-- drivers/bus/mhi/core/debugfs.c | 2 +- drivers/bus/mhi/core/init.c | 72 +++-- drivers/bus/mhi/core/internal.h | 20 +++- drivers/bus/mhi/core/main.c | 416 +++--- drivers/bus/mhi/core/pm.c | 119 - drivers/bus/mhi/pci_generic.c | 330 +- include/linux/mhi.h | 28 +++-- 8 files changed, 775 insertions(+), 276 deletions(-)
Re: [PATCH] clk: qcom: rpmh: add support for SDX55 rpmh IPA clock
On Fri, Apr 09, 2021 at 08:44:07AM -0500, Alex Elder wrote:
> The IPA core clock is required for SDX55. Define it.
>
> Signed-off-by: Alex Elder
I tested this patch on couple of SDX55 based boards like Telit FN980 and
Thundercomm T55. Hence,
Tested-by: Manivannan Sadhasivam
Also cross checked the IPA clock definition using QC internal docs, so
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/clk/qcom/clk-rpmh.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> index c623ce9004063..552d1cbfea4c0 100644
> --- a/drivers/clk/qcom/clk-rpmh.c
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -380,6 +380,7 @@ static const struct clk_rpmh_desc clk_rpmh_sdm845 = {
> DEFINE_CLK_RPMH_VRM(sdx55, rf_clk1, rf_clk1_ao, "rfclkd1", 1);
> DEFINE_CLK_RPMH_VRM(sdx55, rf_clk2, rf_clk2_ao, "rfclkd2", 1);
> DEFINE_CLK_RPMH_BCM(sdx55, qpic_clk, "QP0");
> +DEFINE_CLK_RPMH_BCM(sdx55, ipa, "IP0");
>
> static struct clk_hw *sdx55_rpmh_clocks[] = {
> [RPMH_CXO_CLK] = &sdm845_bi_tcxo.hw,
> @@ -389,6 +390,7 @@ static struct clk_hw *sdx55_rpmh_clocks[] = {
> [RPMH_RF_CLK2] = &sdx55_rf_clk2.hw,
> [RPMH_RF_CLK2_A]= &sdx55_rf_clk2_ao.hw,
> [RPMH_QPIC_CLK] = &sdx55_qpic_clk.hw,
> + [RPMH_IPA_CLK] = &sdx55_ipa.hw,
> };
>
> static const struct clk_rpmh_desc clk_rpmh_sdx55 = {
> --
> 2.27.0
>
Re: [PATCH 2/2] bus: mhi: fix typo in comments for struct mhi_channel_config
On Thu, Apr 08, 2021 at 03:02:20AM -0700, Jarvis Jiang wrote:
> The word 'rung' is a typo in below comment, fix it.
> * @event_ring: The event rung index that services this channel
>
> Signed-off-by: Jarvis Jiang
Applied to mhi-next!
Thanks,
Mani
> ---
> include/linux/mhi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index d095fba37d1e..944aa3aa3035 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -205,7 +205,7 @@ enum mhi_db_brst_mode {
> * @num: The number assigned to this channel
> * @num_elements: The number of elements that can be queued to this channel
> * @local_elements: The local ring length of the channel
> - * @event_ring: The event rung index that services this channel
> + * @event_ring: The event ring index that services this channel
> * @dir: Direction that data may flow on this channel
> * @type: Channel type
> * @ee_mask: Execution Environment mask for this channel
> --
> 2.25.1
>
Re: [PATCH 2/2] bus: mhi: fix typo in comments for struct mhi_channel_config
On Thu, Apr 08, 2021 at 03:02:20AM -0700, Jarvis Jiang wrote:
> The word 'rung' is a typo in below comment, fix it.
> * @event_ring: The event rung index that services this channel
>
> Signed-off-by: Jarvis Jiang
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> include/linux/mhi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index d095fba37d1e..944aa3aa3035 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -205,7 +205,7 @@ enum mhi_db_brst_mode {
> * @num: The number assigned to this channel
> * @num_elements: The number of elements that can be queued to this channel
> * @local_elements: The local ring length of the channel
> - * @event_ring: The event rung index that services this channel
> + * @event_ring: The event ring index that services this channel
> * @dir: Direction that data may flow on this channel
> * @type: Channel type
> * @ee_mask: Execution Environment mask for this channel
> --
> 2.25.1
>
Re: [PATCH] usb: dwc3: qcom: Fixed an issue that the ret value is incorrect in dwc3_qcom_probe()
On Fri, Apr 09, 2021 at 08:49:45AM +0800, Bixuan Cui wrote:
> There is a error message after devm_ioremap_resource failed, and the ret
> is needs to be obtained through PTR_ERR(qcom->qscratch_base).
> We need to move the dev_err() downwards to ensure that the ret value is
> correct.
>
> Fixes: a4333c3a6ba9 ('usb: dwc3: Add Qualcomm DWC3 glue driver')
> Reported-by: Hulk Robot
> Signed-off-by: Bixuan Cui
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index e37cc58dfa55..4716ca8c753d 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -774,8 +774,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>
> qcom->qscratch_base = devm_ioremap_resource(dev, parent_res);
> if (IS_ERR(qcom->qscratch_base)) {
> - dev_err(dev, "failed to map qscratch, err=%d\n", ret);
> ret = PTR_ERR(qcom->qscratch_base);
> + dev_err(dev, "failed to map qscratch, err=%d\n", ret);
But this error message can be removed altogether as devm_ioremap_resource()
reports it already.
Thanks,
Mani
> goto clk_disable;
> }
>
> --
> 2.17.1
>
Re: [PATCH -next] spi: qup: fix PM reference leak in spi_qup_remove()
On Fri, Apr 09, 2021 at 09:54:58AM +, Wang Li wrote: > pm_runtime_get_sync will increment pm usage counter even it failed. > Forgetting to putting operation will result in reference leak here. > Fix it by replacing it with pm_runtime_resume_and_get to keep usage > counter balanced. > > Reported-by: Hulk Robot > Signed-off-by: Wang Li Reviewed-by: Manivannan Sadhasivam Thanks, Mani > --- > drivers/spi/spi-qup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c > index 8dcb2e70735c..d39dec6d1c91 100644 > --- a/drivers/spi/spi-qup.c > +++ b/drivers/spi/spi-qup.c > @@ -1263,7 +1263,7 @@ static int spi_qup_remove(struct platform_device *pdev) > struct spi_qup *controller = spi_master_get_devdata(master); > int ret; > > - ret = pm_runtime_get_sync(&pdev->dev); > + ret = pm_runtime_resume_and_get(&pdev->dev); > if (ret < 0) > return ret; > > -- > 2.17.1 >
Re: [PATCH V2 2/2] soc: qcom: aoss: Add debugfs entry
On Fri, Apr 09, 2021 at 10:09:48AM +0530, Deepak Kumar Singh wrote:
> It can be useful to control the different power states of various
> parts of hardware for device testing. Add a debugfs node for qmp so
> messages can be sent to aoss for debugging and testing purposes.
>
> Signed-off-by: Chris Lew
> Signed-off-by: Deepak Kumar Singh
> ---
> drivers/soc/qcom/qcom_aoss.c | 41 +
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> index 0e397a7..6057bbe 100644
> --- a/drivers/soc/qcom/qcom_aoss.c
> +++ b/drivers/soc/qcom/qcom_aoss.c
> @@ -4,6 +4,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -88,6 +89,9 @@ struct qmp {
> struct clk_hw qdss_clk;
> struct genpd_onecell_data pd_data;
> struct qmp_cooling_device *cooling_devs;
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + struct dentry *debugfs_file;
> +#endif /* CONFIG_DEBUG_FS */
> };
>
> struct qmp_pd {
> @@ -560,6 +564,34 @@ void qmp_put(struct platform_device *pdev)
> }
> EXPORT_SYMBOL(qmp_put);
>
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> +static ssize_t aoss_dbg_write(struct file *file, const char __user *userstr,
> + size_t len, loff_t *pos)
> +{
> + struct qmp *qmp = file->private_data;
> + char buf[QMP_MSG_LEN] = {};
> + int ret;
> +
> + if (!len || len >= QMP_MSG_LEN)
> + return -EINVAL;
> +
> + ret = copy_from_user(buf, userstr, len);
> + if (ret) {
> + dev_err(qmp->dev, "copy from user failed, ret:%d\n", ret);
Does the userspace need to know how many bytes were not copied? I don't
think this is a useful information. So you could remove this err print.
With that,
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> + return -EFAULT;
> + }
> +
> + ret = qmp_send(qmp, buf, QMP_MSG_LEN);
> +
> + return ret ? ret : len;
> +}
> +
> +static const struct file_operations aoss_dbg_fops = {
> + .open = simple_open,
> + .write = aoss_dbg_write,
> +};
> +#endif /* CONFIG_DEBUG_FS */
> +
> static int qmp_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -616,6 +648,11 @@ static int qmp_probe(struct platform_device *pdev)
>
> atomic_set(&qmp->orphan, 0);
>
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + qmp->debugfs_file = debugfs_create_file("aoss_send_message", 0220, NULL,
> + qmp, &aoss_dbg_fops);
> +#endif /* CONFIG_DEBUG_FS */
> +
> return 0;
>
> err_remove_qdss_clk:
> @@ -632,6 +669,10 @@ static int qmp_remove(struct platform_device *pdev)
> {
> struct qmp *qmp = platform_get_drvdata(pdev);
>
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + debugfs_remove(qmp->debugfs_file);
> +#endif /* CONFIG_DEBUG_FS */
> +
> qmp_qdss_clk_remove(qmp);
> qmp_pd_remove(qmp);
> qmp_cooling_devices_remove(qmp);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH V1 1/2] soc: qcom: aoss: Expose send for generic usecase
On Sun, Apr 04, 2021 at 12:17:52PM -0500, Bjorn Andersson wrote:
> On Fri 02 Apr 01:17 CDT 2021, Deepak Kumar Singh wrote:
>
> > Not all upcoming usecases will have an interface to allow the aoss
> > driver to hook onto. Expose the send api and create a get function to
> > enable drivers to send their own messages to aoss.
> >
> > Signed-off-by: Chris Lew
> > Signed-off-by: Deepak Kumar Singh
> > ---
> > drivers/soc/qcom/qcom_aoss.c | 36 +++-
> > 1 file changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> > index 53acb94..5c643f0 100644
> > --- a/drivers/soc/qcom/qcom_aoss.c
> > +++ b/drivers/soc/qcom/qcom_aoss.c
> > @@ -8,10 +8,12 @@
[...]
> > + pdev = of_find_device_by_node(np);
>
> of_find_device_by_node() will increment the refcount of the underlying
> struct device of pdev, so you need to platform_device_put() once you're
> done with it.
>
> As a side effect of not putting the struct device, the devm_kzalloc'ed
> qmp pointer will remain valid. So care is needed to make sure that the
> client doesn't end up with a dangling pointer if the qmp device is
> removed.
>
> My suggestion is that you add a "qmp_put()" function, which invokes
> platform_device_put() and that you add some sort of tracking ("bool
> orphan"?) to the struct qmp and make qmp_send() fail if this is set.
>
I think this is a duplication of what the struct device offers. Why
can't we use the generic infrastructure for this usecase?
Like using device_initialize() in qmp_probe() along with a release
callback for "struct device", then using get_device() in qmp_get().
Then there should also be a qmp_put() API which calls put_device() to
decrease the refcount.
Ideally, the final refcount should be dropped in qmp_remove() and then
the release callback will be called automatically to free "struct qmp".
> That way if someone unbinds the aoss device, the client will still have
> a "valid" pointer, but won't be able to qmp_send() after qmp_close() has
> been called in the aoss remove function.
>
How can someone remove the qmp device if a client is holding its reference?
Thanks,
Mani
> Regards,
> Bjorn
>
> > + if (!pdev)
> > + return ERR_PTR(-EINVAL);
> > +
> > + qmp = platform_get_drvdata(pdev);
> > + return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
> > +}
> > +EXPORT_SYMBOL(qmp_get);
> > +
> > static int qmp_probe(struct platform_device *pdev)
> > {
> > struct resource *res;
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
Re: [PATCH -next] mmc: sdhci-msm: Remove unnecessary error log
On Fri, Apr 09, 2021 at 09:54:24AM +0800, Jia Yang wrote:
> devm_ioremap_resource() has recorded error log, so it's
> unnecessary to record log again.
>
> Reported-by: Hulk Robot
> Signed-off-by: Jia Yang
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/mmc/host/sdhci-msm.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index d170c919e6e4..e44b7a66b73c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1863,7 +1863,6 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host
> *msm_host,
> struct mmc_host *mmc = msm_host->mmc;
> struct device *dev = mmc_dev(mmc);
> struct resource *res;
> - int err;
>
> if (!(cqhci_readl(cq_host, CQHCI_CAP) & CQHCI_CAP_CS))
> return 0;
> @@ -1881,11 +1880,8 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host
> *msm_host,
> }
>
> msm_host->ice_mem = devm_ioremap_resource(dev, res);
> - if (IS_ERR(msm_host->ice_mem)) {
> - err = PTR_ERR(msm_host->ice_mem);
> - dev_err(dev, "Failed to map ICE registers; err=%d\n", err);
> - return err;
> - }
> + if (IS_ERR(msm_host->ice_mem))
> + return PTR_ERR(msm_host->ice_mem);
>
> if (!sdhci_msm_ice_supported(msm_host))
> goto disable;
> --
> 2.25.1
>
Re: [PATCH -next] clocksource/drivers/qcom: add missing iounmap() on error in msm_dt_timer_init()
On Fri, Apr 09, 2021 at 12:56:57PM +0800, Yang Yingliang wrote:
> base and cpu0_base are not unmapped on error path, add the missing
> iounmap() before return msm_dt_timer_init() in the error handling
> cases.
>
> Fixes: 6e3321631ac2 ("ARM: msm: Add DT support to msm_timer")
> Reported-by: Hulk Robot
> Signed-off-by: Yang Yingliang
> ---
> drivers/clocksource/timer-qcom.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clocksource/timer-qcom.c
> b/drivers/clocksource/timer-qcom.c
> index b4afe3a67583..3488876198e0 100644
> --- a/drivers/clocksource/timer-qcom.c
> +++ b/drivers/clocksource/timer-qcom.c
> @@ -213,7 +213,8 @@ static int __init msm_dt_timer_init(struct device_node
> *np)
> irq = irq_of_parse_and_map(np, 1);
> if (irq <= 0) {
> pr_err("Can't get irq\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto err_unmap_base;
> }
>
> /* We use CPU0's DGT for the clocksource */
> @@ -223,18 +224,19 @@ static int __init msm_dt_timer_init(struct device_node
> *np)
> ret = of_address_to_resource(np, 0, &res);
> if (ret) {
> pr_err("Failed to parse DGT resource\n");
> - return ret;
> + goto err_unmap_base;
> }
>
> cpu0_base = ioremap(res.start + percpu_offset, resource_size(&res));
> if (!cpu0_base) {
> pr_err("Failed to map source base\n");
> - return -EINVAL;
Missing "ret = -EINVAL" assignment. With that fixed,
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> + goto err_unmap_base;
> }
>
> if (of_property_read_u32(np, "clock-frequency", &freq)) {
> pr_err("Unknown frequency\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto err_unmap_cpu0_base;
> }
>
> event_base = base + 0x4;
> @@ -243,7 +245,18 @@ static int __init msm_dt_timer_init(struct device_node
> *np)
> freq /= 4;
> writel_relaxed(DGT_CLK_CTL_DIV_4, source_base + DGT_CLK_CTL);
>
> - return msm_timer_init(freq, 32, irq, !!percpu_offset);
> + ret = msm_timer_init(freq, 32, irq, !!percpu_offset);
> + if (ret)
> + goto err_unmap_cpu0_base;
> +
> + return 0;
> +
> +err_unmap_cpu0_base:
> + iounmap(cpu0_base);
> +err_unmap_base:
> + iounmap(base);
> +
> + return ret;
> }
> TIMER_OF_DECLARE(kpss_timer, "qcom,kpss-timer", msm_dt_timer_init);
> TIMER_OF_DECLARE(scss_timer, "qcom,scss-timer", msm_dt_timer_init);
> --
> 2.25.1
>
Re: [PATCH -next] clk: qcom: Add missing MODULE_DEVICE_TABLE
On Thu, Apr 08, 2021 at 09:55:09PM +0800, Chen Hui wrote:
> Add missing MODULE_DEVICE_TABLE entries to support module autoloading,
> as these drivers can be compiled as external modules.
>
> Signed-off-by: Chen Hui
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/clk/qcom/a53-pll.c | 1 +
> drivers/clk/qcom/a7-pll.c | 1 +
> drivers/clk/qcom/apss-ipq-pll.c | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> index 45cfc57bff92..af6ac17c7dae 100644
> --- a/drivers/clk/qcom/a53-pll.c
> +++ b/drivers/clk/qcom/a53-pll.c
> @@ -93,6 +93,7 @@ static const struct of_device_id qcom_a53pll_match_table[]
> = {
> { .compatible = "qcom,msm8916-a53pll" },
> { }
> };
> +MODULE_DEVICE_TABLE(of, qcom_a53pll_match_table);
>
> static struct platform_driver qcom_a53pll_driver = {
> .probe = qcom_a53pll_probe,
> diff --git a/drivers/clk/qcom/a7-pll.c b/drivers/clk/qcom/a7-pll.c
> index e171d3caf2cf..c4a53e5db229 100644
> --- a/drivers/clk/qcom/a7-pll.c
> +++ b/drivers/clk/qcom/a7-pll.c
> @@ -86,6 +86,7 @@ static const struct of_device_id qcom_a7pll_match_table[] =
> {
> { .compatible = "qcom,sdx55-a7pll" },
> { }
> };
> +MODULE_DEVICE_TABLE(of, qcom_a7pll_match_table);
>
> static struct platform_driver qcom_a7pll_driver = {
> .probe = qcom_a7pll_probe,
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index 30be87fb222a..bef7899ad0d6 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -81,6 +81,7 @@ static const struct of_device_id apss_ipq_pll_match_table[]
> = {
> { .compatible = "qcom,ipq6018-a53pll" },
> { }
> };
> +MODULE_DEVICE_TABLE(of, apss_ipq_pll_match_table);
>
> static struct platform_driver apss_ipq_pll_driver = {
> .probe = apss_ipq_pll_probe,
> --
> 2.17.1
>
Re: [PATCH -next] mmc: owl-mmc: Remove unnecessary error log
On Fri, Apr 09, 2021 at 10:33:49AM +0800, Laibin Qiu wrote:
> devm_ioremap_resource() has recorded error log, so it's
> unnecessary to record log again.
>
> Reported-by: Hulk Robot
> Signed-off-by: Laibin Qiu
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/mmc/host/owl-mmc.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mmc/host/owl-mmc.c b/drivers/mmc/host/owl-mmc.c
> index 5490962dc8e5..3dc143b03939 100644
> --- a/drivers/mmc/host/owl-mmc.c
> +++ b/drivers/mmc/host/owl-mmc.c
> @@ -581,7 +581,6 @@ static int owl_mmc_probe(struct platform_device *pdev)
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> owl_host->base = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(owl_host->base)) {
> - dev_err(&pdev->dev, "Failed to remap registers\n");
> ret = PTR_ERR(owl_host->base);
> goto err_free_host;
> }
> --
> 2.25.1
>
[PATCH v3] bus: mhi: core: Fix shadow declarations
This commit fixes below sparse warnings with W=2 about shadow
declarations:
drivers/bus/mhi/core/main.c: In function ‘parse_xfer_event’:
drivers/bus/mhi/core/main.c:667:17: warning: declaration of ‘flags’ shadows a
previous local [-Wshadow]
667 | unsigned long flags;
| ^
drivers/bus/mhi/core/main.c:565:16: note: shadowed declaration is here
565 | unsigned long flags = 0;
|^
drivers/bus/mhi/core/main.c: In function ‘mhi_process_ctrl_ev_ring’:
drivers/bus/mhi/core/main.c:856:23: warning: declaration of ‘new_state’ shadows
a previous local [-Wshadow]
856 | enum mhi_pm_state new_state;
| ^
drivers/bus/mhi/core/main.c:837:19: note: shadowed declaration is here
837 |enum mhi_state new_state;
| ^
Signed-off-by: Manivannan Sadhasivam
---
Changes in v3:
* Fixed the usage of "flags" by renaming to "pm_lock_flags"
drivers/bus/mhi/core/main.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index b0c8afe16e3a..22acde118bc3 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -664,15 +664,15 @@ static int parse_xfer_event(struct mhi_controller
*mhi_cntrl,
case MHI_EV_CC_OOB:
case MHI_EV_CC_DB_MODE:
{
- unsigned long flags;
+ unsigned long pm_lock_flags;
mhi_chan->db_cfg.db_mode = 1;
- read_lock_irqsave(&mhi_cntrl->pm_lock, flags);
+ read_lock_irqsave(&mhi_cntrl->pm_lock, pm_lock_flags);
if (tre_ring->wp != tre_ring->rp &&
MHI_DB_ACCESS_VALID(mhi_cntrl)) {
mhi_ring_chan_db(mhi_cntrl, mhi_chan);
}
- read_unlock_irqrestore(&mhi_cntrl->pm_lock, flags);
+ read_unlock_irqrestore(&mhi_cntrl->pm_lock, pm_lock_flags);
break;
}
case MHI_EV_CC_BAD_TRE:
@@ -853,14 +853,14 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
break;
case MHI_STATE_SYS_ERR:
{
- enum mhi_pm_state new_state;
+ enum mhi_pm_state pm_state;
dev_dbg(dev, "System error detected\n");
write_lock_irq(&mhi_cntrl->pm_lock);
- new_state = mhi_tryset_pm_state(mhi_cntrl,
+ pm_state = mhi_tryset_pm_state(mhi_cntrl,
MHI_PM_SYS_ERR_DETECT);
write_unlock_irq(&mhi_cntrl->pm_lock);
- if (new_state == MHI_PM_SYS_ERR_DETECT)
+ if (pm_state == MHI_PM_SYS_ERR_DETECT)
mhi_pm_sys_err_handler(mhi_cntrl);
break;
}
--
2.25.1
[PATCH] remoteproc: qcom: pas: Add modem support for SDX55
Add remoteproc support for Hexagon modem found on the Qualcomm SDX55
platform.
Signed-off-by: Manivannan Sadhasivam
---
drivers/remoteproc/qcom_q6v5_pas.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c
b/drivers/remoteproc/qcom_q6v5_pas.c
index e635454d6170..292141877260 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -785,6 +785,22 @@ static const struct adsp_data wcss_resource_init = {
.ssctl_id = 0x12,
};
+static const struct adsp_data sdx55_mpss_resource = {
+ .crash_reason_smem = 421,
+ .firmware_name = "modem.mdt",
+ .pas_id = 4,
+ .has_aggre2_clk = false,
+ .auto_boot = true,
+ .proxy_pd_names = (char*[]){
+ "cx",
+ "mss",
+ NULL
+ },
+ .ssr_name = "mpss",
+ .sysmon_name = "modem",
+ .ssctl_id = 0x22,
+};
+
static const struct of_device_id adsp_of_match[] = {
{ .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init},
{ .compatible = "qcom,msm8996-adsp-pil", .data = &adsp_resource_init},
@@ -797,6 +813,7 @@ static const struct of_device_id adsp_of_match[] = {
{ .compatible = "qcom,sc7180-mpss-pas", .data = &mpss_resource_init},
{ .compatible = "qcom,sdm845-adsp-pas", .data = &adsp_resource_init},
{ .compatible = "qcom,sdm845-cdsp-pas", .data = &cdsp_resource_init},
+ { .compatible = "qcom,sdx55-mpss-pas", .data = &sdx55_mpss_resource},
{ .compatible = "qcom,sm8150-adsp-pas", .data = &sm8150_adsp_resource},
{ .compatible = "qcom,sm8150-cdsp-pas", .data = &sm8150_cdsp_resource},
{ .compatible = "qcom,sm8150-mpss-pas", .data = &mpss_resource_init},
--
2.25.1
[PATCH 4/7] ARM: configs: qcom_defconfig: Enable Q6V5_PAS remoteproc driver
Enable the Qualcomm Q6V5_PAS (Peripheral Authentication Service) remoteproc driver to manage the modem co-processor in SDX55 platform. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 47343d0ea586..695612829503 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -237,6 +237,7 @@ CONFIG_MAILBOX=y CONFIG_QCOM_APCS_IPC=y CONFIG_REMOTEPROC=y CONFIG_QCOM_ADSP_PIL=y +CONFIG_QCOM_Q6V5_PAS=y CONFIG_QCOM_Q6V5_PIL=y CONFIG_QCOM_WCNSS_PIL=y CONFIG_RPMSG_CHAR=y -- 2.25.1
[PATCH 7/7] ARM: configs: qcom_defconfig: Reduce CMA size to 64MB
Not all platforms are able to allocate CMA size of 256MB. One such platform is SDX55. Hence, use the standard 64MB size for CMA. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 4e4c49c29aa5..26353cbfa968 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -296,7 +296,7 @@ CONFIG_NLS_ASCII=y CONFIG_NLS_ISO8859_1=y CONFIG_NLS_UTF8=y CONFIG_DMA_CMA=y -CONFIG_CMA_SIZE_MBYTES=256 +CONFIG_CMA_SIZE_MBYTES=64 CONFIG_PRINTK_TIME=y CONFIG_DYNAMIC_DEBUG=y CONFIG_DEBUG_INFO=y -- 2.25.1
[PATCH 6/7] ARM: configs: qcom_defconfig: Enable GLINK SMEM driver
Enable the Qualcomm GLINK SMEM driver to support GLINK protocol over shared memory. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 5955aeb0646e..4e4c49c29aa5 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -241,6 +241,7 @@ CONFIG_QCOM_Q6V5_PAS=y CONFIG_QCOM_Q6V5_PIL=y CONFIG_QCOM_WCNSS_PIL=y CONFIG_RPMSG_CHAR=y +CONFIG_RPMSG_QCOM_GLINK_SMEM=y CONFIG_RPMSG_QCOM_SMD=y CONFIG_QCOM_COMMAND_DB=y CONFIG_QCOM_GSBI=y -- 2.25.1
[PATCH 5/7] ARM: configs: qcom_defconfig: Enable SDX55 interconnect driver
Enable interconnect driver for SDX55 platform to manage the interconnect providers. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 695612829503..5955aeb0646e 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -277,6 +277,7 @@ CONFIG_QCOM_QFPROM=y CONFIG_INTERCONNECT=y CONFIG_INTERCONNECT_QCOM=y CONFIG_INTERCONNECT_QCOM_MSM8974=m +CONFIG_INTERCONNECT_QCOM_SDX55=m CONFIG_EXT2_FS=y CONFIG_EXT2_FS_XATTR=y CONFIG_EXT3_FS=y -- 2.25.1
[PATCH 3/7] ARM: configs: qcom_defconfig: Enable CPUFreq support
Enable CPUFreq and CPUFreq DT drivers to carry out CPU Frequency scaling duties on platforms like SDX55. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 02f6185f31a6..47343d0ea586 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -302,3 +302,5 @@ CONFIG_MAGIC_SYSRQ=y CONFIG_WATCHDOG=y CONFIG_QCOM_WDT=y CONFIG_ARM_PSCI=y +CONFIG_CPU_FREQ=y +CONFIG_CPUFREQ_DT=y -- 2.25.1
[PATCH 2/7] ARM: configs: qcom_defconfig: Enable SDX55 A7 PLL and APCS clock driver
Enable A7 PLL driver and APCS clock driver on SDX55 platform. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 0b9da27f923a..02f6185f31a6 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -215,6 +215,8 @@ CONFIG_DMADEVICES=y CONFIG_QCOM_BAM_DMA=y CONFIG_STAGING=y CONFIG_COMMON_CLK_QCOM=y +CONFIG_QCOM_A7PLL=y +CONFIG_QCOM_CLK_APCS_SDX55=y CONFIG_QCOM_CLK_RPM=y CONFIG_QCOM_CLK_RPMH=y CONFIG_QCOM_CLK_SMD_RPM=y -- 2.25.1
[PATCH 1/7] ARM: configs: qcom_defconfig: Enable APCS IPC mailbox driver
Enable Qualcomm APCS IPC mailbox driver for IPC communication between application processor and other masters in platforms like SDX55. Signed-off-by: Manivannan Sadhasivam --- arch/arm/configs/qcom_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 3f36887e8333..0b9da27f923a 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -232,6 +232,7 @@ CONFIG_ARM_SMMU=y CONFIG_HWSPINLOCK=y CONFIG_HWSPINLOCK_QCOM=y CONFIG_MAILBOX=y +CONFIG_QCOM_APCS_IPC=y CONFIG_REMOTEPROC=y CONFIG_QCOM_ADSP_PIL=y CONFIG_QCOM_Q6V5_PIL=y -- 2.25.1
[PATCH 0/7] SDX55 defconfig updates for v5.13
Hi Bjorn, This series updates the qcom_defconfig by enabling the drivers required for the SDX55 platform. Please consider merging! Thanks, Mani Manivannan Sadhasivam (7): ARM: configs: qcom_defconfig: Enable APCS IPC mailbox driver ARM: configs: qcom_defconfig: Enable SDX55 A7 PLL and APCS clock driver ARM: configs: qcom_defconfig: Enable CPUFreq support ARM: configs: qcom_defconfig: Enable Q6V5_PAS remoteproc driver ARM: configs: qcom_defconfig: Enable SDX55 interconnect driver ARM: configs: qcom_defconfig: Enable GLINK SMEM driver ARM: configs: qcom_defconfig: Reduce CMA size to 64MB arch/arm/configs/qcom_defconfig | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.25.1
[PATCH 15/15] ARM: dts: qcom: sdx55: Add Modem remoteproc node
Add modem support to SDX55 using the PAS remoteproc driver.
Signed-off-by: Manivannan Sadhasivam
---
.../boot/dts/qcom-sdx55-telit-fn980-tlb.dts | 5 +++
arch/arm/boot/dts/qcom-sdx55.dtsi | 33 +++
2 files changed, 38 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
index 6da366ec15b3..3065f84634b8 100644
--- a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
+++ b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
@@ -255,6 +255,11 @@ nand@0 {
};
};
+&remoteproc_mpss {
+ status = "okay";
+ memory-region = <&mpss_adsp_mem>;
+};
+
&usb_hsphy {
status = "okay";
vdda-pll-supply = <&vreg_l4e_bb_0p875>;
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index aa3edecf5810..bed83d1ddc29 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -328,6 +328,39 @@ sdhc_1: sdhci@8804000 {
status = "disabled";
};
+ remoteproc_mpss: remoteproc@408 {
+ compatible = "qcom,sdx55-mpss-pas";
+ reg = <0x0408 0x4040>;
+
+ interrupts-extended = <&intc GIC_SPI 250
IRQ_TYPE_EDGE_RISING>,
+ <&modem_smp2p_in 0
IRQ_TYPE_EDGE_RISING>,
+ <&modem_smp2p_in 1
IRQ_TYPE_EDGE_RISING>,
+ <&modem_smp2p_in 2
IRQ_TYPE_EDGE_RISING>,
+ <&modem_smp2p_in 3
IRQ_TYPE_EDGE_RISING>,
+ <&modem_smp2p_in 7
IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "wdog", "fatal", "ready", "handover",
+ "stop-ack", "shutdown-ack";
+
+ clocks = <&rpmhcc RPMH_CXO_CLK>;
+ clock-names = "xo";
+
+ power-domains = <&rpmhpd SDX55_CX>,
+ <&rpmhpd SDX55_MSS>;
+ power-domain-names = "cx", "mss";
+
+ qcom,smem-states = <&modem_smp2p_out 0>;
+ qcom,smem-state-names = "stop";
+
+ status = "disabled";
+
+ glink-edge {
+ interrupts = ;
+ label = "mpss";
+ qcom,remote-pid = <1>;
+ mboxes = <&apcs 15>;
+ };
+ };
+
usb: usb@a6f8800 {
compatible = "qcom,sdx55-dwc3", "qcom,dwc3";
reg = <0x0a6f8800 0x400>;
--
2.25.1
[PATCH 14/15] dt-bindings: remoteproc: qcom: pas: Add binding for SDX55
Add devicetree binding for SDX55 remoteproc. Cc: Rob Herring Cc: devicet...@vger.kernel.org Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt index 1c330a8941f9..229f908fd831 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt @@ -18,6 +18,7 @@ on the Qualcomm ADSP Hexagon core. "qcom,sc7180-mpss-pas" "qcom,sdm845-adsp-pas" "qcom,sdm845-cdsp-pas" +"qcom,sdx55-mpss-pas" "qcom,sm8150-adsp-pas" "qcom,sm8150-cdsp-pas" "qcom,sm8150-mpss-pas" @@ -61,6 +62,7 @@ on the Qualcomm ADSP Hexagon core. must be "wdog", "fatal", "ready", "handover", "stop-ack" qcom,qcs404-wcss-pas: qcom,sc7180-mpss-pas: +qcom,sdx55-mpss-pas: qcom,sm8150-mpss-pas: qcom,sm8350-mpss-pas: must be "wdog", "fatal", "ready", "handover", "stop-ack", @@ -128,6 +130,8 @@ on the Qualcomm ADSP Hexagon core. qcom,sm8150-mpss-pas: qcom,sm8350-mpss-pas: must be "cx", "load_state", "mss" +qcom,sdx55-mpss-pas: +must be "cx", "mss" qcom,sm8250-adsp-pas: qcom,sm8350-adsp-pas: qcom,sm8150-slpi-pas: -- 2.25.1
[PATCH 13/15] ARM: dts: qcom: sdx55: Add basic devicetree support for Thundercomm T55
Thundercomm T55 is the development platform based on the Qualcomm SDX55
chipset. This basic support includes support for debug serial, NAND
flash, BAM DMA, USB and regulators support.
https://www.thundercomm.com/app_en/product/1593506006365532
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/Makefile | 3 +-
arch/arm/boot/dts/qcom-sdx55-t55.dts | 281 +++
2 files changed, 283 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/qcom-sdx55-t55.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 874ab2b66187..1d314fdf6014 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -930,7 +930,8 @@ dtb-$(CONFIG_ARCH_QCOM) += \
qcom-msm8974-sony-xperia-honami.dtb \
qcom-mdm9615-wp8548-mangoh-green.dtb \
qcom-sdx55-mtp.dtb \
- qcom-sdx55-telit-fn980-tlb.dtb
+ qcom-sdx55-telit-fn980-tlb.dtb \
+ qcom-sdx55-t55.dtb
dtb-$(CONFIG_ARCH_RDA) += \
rda8810pl-orangepi-2g-iot.dtb \
rda8810pl-orangepi-i96.dtb
diff --git a/arch/arm/boot/dts/qcom-sdx55-t55.dts
b/arch/arm/boot/dts/qcom-sdx55-t55.dts
new file mode 100644
index ..ddcd53aa533d
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-sdx55-t55.dts
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2021, Linaro Ltd.
+ */
+
+/dts-v1/;
+
+#include
+#include
+#include "qcom-sdx55.dtsi"
+#include "qcom-pmx55.dtsi"
+
+/ {
+ model = "Thundercomm T55 Development Kit";
+ compatible = "qcom,sdx55-t55", "qcom,sdx55";
+ qcom,board-id = <0xb010008 0x4>;
+
+ aliases {
+ serial0 = &blsp1_uart3;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ mpss_debug_mem: memory@8ef0 {
+ no-map;
+ reg = <0x8ef0 0x80>;
+ };
+
+ ipa_fw_mem: memory@8fced000 {
+ no-map;
+ reg = <0x8fced000 0x1>;
+ };
+
+ mpss_adsp_mem: memory@9080 {
+ no-map;
+ reg = <0x9080 0xf80>;
+ };
+ };
+
+ vph_pwr: vph-pwr-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vph_pwr";
+ regulator-min-microvolt = <370>;
+ regulator-max-microvolt = <370>;
+ };
+
+ vreg_bob_3p3: pmx55-bob {
+ compatible = "regulator-fixed";
+ regulator-name = "vreg_bob_3p3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+
+ regulator-always-on;
+ regulator-boot-on;
+
+ vin-supply = <&vph_pwr>;
+ };
+
+ vreg_s7e_mx_0p752: pmx55-s7e {
+ compatible = "regulator-fixed";
+ regulator-name = "vreg_s7e_mx_0p752";
+ regulator-min-microvolt = <752000>;
+ regulator-max-microvolt = <752000>;
+
+ vin-supply = <&vph_pwr>;
+ };
+
+ vreg_sd_vdd: sd-vdd {
+ compatible = "regulator-fixed";
+ regulator-name = "vreg_sd_vdd";
+ regulator-min-microvolt = <295>;
+ regulator-max-microvolt = <295>;
+
+ vin-supply = <&vreg_vddpx_2>;
+ };
+
+ vreg_vddpx_2: vddpx-2 {
+ compatible = "regulator-gpio";
+ regulator-name = "vreg_vddpx_2";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <285>;
+ enable-gpios = <&tlmm 98 GPIO_ACTIVE_HIGH>;
+ gpios = <&tlmm 100 GPIO_ACTIVE_HIGH>;
+ states = <180 0>, <285 1>;
+ startup-delay-us = <20>;
+ enable-active-high;
+ regulator-boot-on;
+
+ vin-supply = <&vph_pwr>;
+ };
+};
+
+&apps_rsc {
+ pmx55-rpmh-regulators {
+ compatible = "qcom,pmx55-rpmh-regulators";
+ qcom,pmic-id = "e";
+
+ vdd-s1-supply = <&vph_pwr>;
+ vdd-s2-supply = <&vph_pwr>;
+ vdd-s3-supply = <&vph_pwr>;
+ vdd-s4-supply = <&vph_pwr>;
+ vdd-s5-supply = <&vph_pwr>;
+ vdd-s6-supply = <&vph_pwr>;
+
[PATCH 12/15] dt-bindings: arm: qcom: Add binding for Thundercomm T55 kit
Add devicetree binding for Thundercomm T55 Dev kit based on SDX55. Cc: Rob Herring Cc: devicet...@vger.kernel.org Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/arm/qcom.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml index d5d561d9e8aa..6827bf82fb74 100644 --- a/Documentation/devicetree/bindings/arm/qcom.yaml +++ b/Documentation/devicetree/bindings/arm/qcom.yaml @@ -173,6 +173,7 @@ properties: - enum: - qcom,sdx55-mtp - qcom,sdx55-telit-fn980-tlb + - qcom,sdx55-t55 - const: qcom,sdx55 - items: -- 2.25.1
[PATCH 11/15] ARM: dts: qcom: sdx55: Add basic devicetree support for Telit FN980 TLB
Telit FN980 TLB is the development platform based on the Qualcomm SDX55
chipset. This basic support includes support for debug serial, NAND
flash, BAM DMA, USB and regulators support.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/Makefile| 3 +-
.../boot/dts/qcom-sdx55-telit-fn980-tlb.dts | 277 ++
2 files changed, 279 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 8e5d4ab4e75e..874ab2b66187 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -929,7 +929,8 @@ dtb-$(CONFIG_ARCH_QCOM) += \
qcom-msm8974-sony-xperia-castor.dtb \
qcom-msm8974-sony-xperia-honami.dtb \
qcom-mdm9615-wp8548-mangoh-green.dtb \
- qcom-sdx55-mtp.dtb
+ qcom-sdx55-mtp.dtb \
+ qcom-sdx55-telit-fn980-tlb.dtb
dtb-$(CONFIG_ARCH_RDA) += \
rda8810pl-orangepi-2g-iot.dtb \
rda8810pl-orangepi-i96.dtb
diff --git a/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
new file mode 100644
index ..6da366ec15b3
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2021, Linaro Ltd.
+ */
+
+/dts-v1/;
+
+#include
+#include
+#include "qcom-sdx55.dtsi"
+#include "qcom-pmx55.dtsi"
+
+/ {
+ model = "Telit FN980 TLB";
+ compatible = "qcom,sdx55-telit-fn980-tlb", "qcom,sdx55";
+ qcom,board-id = <0xb010008 0x0>;
+
+ aliases {
+ serial0 = &blsp1_uart3;
+ };
+
+ chosen {
+ stdout-path = "serial0:921600n8";
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ mpss_debug_mem: memory@8ef0 {
+ no-map;
+ reg = <0x8ef0 0x80>;
+ };
+
+ ipa_fw_mem: memory@8fced000 {
+ no-map;
+ reg = <0x8fced000 0x1>;
+ };
+
+ mpss_adsp_mem: memory@9080 {
+ no-map;
+ reg = <0x9080 0xf80>;
+ };
+ };
+
+ vph_pwr: vph-pwr-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vph_pwr";
+ regulator-min-microvolt = <370>;
+ regulator-max-microvolt = <370>;
+ };
+
+ vreg_bob_3p3: pmx55-bob {
+ compatible = "regulator-fixed";
+ regulator-name = "vreg_bob_3p3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+
+ regulator-always-on;
+ regulator-boot-on;
+
+ vin-supply = <&vph_pwr>;
+ };
+
+ vreg_s7e_mx_0p752: pmx55-s7e {
+ compatible = "regulator-fixed";
+ regulator-name = "vreg_s7e_mx_0p752";
+ regulator-min-microvolt = <752000>;
+ regulator-max-microvolt = <752000>;
+
+ vin-supply = <&vph_pwr>;
+ };
+
+ vreg_sd_vdd: sd-vdd {
+ compatible = "regulator-fixed";
+ regulator-name = "vreg_sd_vdd";
+ regulator-min-microvolt = <295>;
+ regulator-max-microvolt = <295>;
+
+ vin-supply = <&vreg_vddpx_2>;
+ };
+
+ vreg_vddpx_2: vddpx-2 {
+ compatible = "regulator-gpio";
+ regulator-name = "vreg_vddpx_2";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <285>;
+ enable-gpios = <&tlmm 98 GPIO_ACTIVE_HIGH>;
+ gpios = <&tlmm 100 GPIO_ACTIVE_HIGH>;
+ states = <180 0>, <285 1>;
+ startup-delay-us = <20>;
+ enable-active-high;
+ regulator-boot-on;
+
+ vin-supply = <&vph_pwr>;
+ };
+};
+
+&apps_rsc {
+ pmx55-rpmh-regulators {
+ compatible = "qcom,pmx55-rpmh-regulators";
+ qcom,pmic-id = "e";
+
+ vdd-s1-supply = <&vph_pwr>;
+ vdd-s2-supply = <&vph_pwr>;
+ vdd-s3-supply = <&vph_pwr>;
+ vdd-s4-supply = <&vph_pwr>;
+ vdd-s5-supply = <&vph_pwr>;
+ vdd-s6-supply = <&vph_pwr>;
+
[PATCH 10/15] dt-bindings: arm: qcom: Add binding for Telit FN980 TLB board
Add devicetree binding for Telit FN980 TLB board based on SDX55. Cc: Rob Herring Cc: devicet...@vger.kernel.org Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/arm/qcom.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml index 174134f920e1..d5d561d9e8aa 100644 --- a/Documentation/devicetree/bindings/arm/qcom.yaml +++ b/Documentation/devicetree/bindings/arm/qcom.yaml @@ -172,6 +172,7 @@ properties: - items: - enum: - qcom,sdx55-mtp + - qcom,sdx55-telit-fn980-tlb - const: qcom,sdx55 - items: -- 2.25.1
[PATCH 09/15] ARM: dts: qcom: Fix node name for NAND controller node
Use the common "nand-controller" node name for NAND controller node to
fix the `make dtbs_check` validation for Qcom platforms.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-ipq4019.dtsi | 2 +-
arch/arm/boot/dts/qcom-sdx55.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi
b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index 7bf1da916f25..ff1bdb10ad19 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -480,7 +480,7 @@ qpic_bam: dma@7984000 {
status = "disabled";
};
- nand: qpic-nand@79b {
+ nand: nand-controller@79b {
compatible = "qcom,ipq4019-nand";
reg = <0x79b 0x1000>;
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 3372e076f9bd..aa3edecf5810 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -294,7 +294,7 @@ qpic_bam: dma-controller@1b04000 {
status = "disabled";
};
- qpic_nand: nand@1b3 {
+ qpic_nand: nand-controller@1b3 {
compatible = "qcom,sdx55-nand";
reg = <0x01b3 0x1>;
#address-cells = <1>;
--
2.25.1
[PATCH 08/15] ARM: dts: qcom: sdx55: Add interconnect nodes
Add interconnect nodes for the providers in SDX55 platform.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 33 +++
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index daf34f24a5d3..3372e076f9bd 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
#include
@@ -253,6 +254,34 @@ usb_ssphy: phy@ff6200 {
};
};
+ mc_virt: interconnect@110 {
+ compatible = "qcom,sdx55-mc-virt";
+ reg = <0x0110 0x40>;
+ #interconnect-cells = <1>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ };
+
+ mem_noc: interconnect@968 {
+ compatible = "qcom,sdx55-mem-noc";
+ reg = <0x0968 0x4>;
+ #interconnect-cells = <1>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ };
+
+ system_noc: interconnect@162c000 {
+ compatible = "qcom,sdx55-system-noc";
+ reg = <0x0162c000 0x31200>;
+ #interconnect-cells = <1>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ };
+
+ ipa_virt: interconnect@1e0 {
+ compatible = "qcom,sdx55-ipa-virt";
+ reg = <0x01e0 0x10>;
+ #interconnect-cells = <1>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ };
+
qpic_bam: dma-controller@1b04000 {
compatible = "qcom,bam-v1.7.0";
reg = <0x01b04000 0x1c000>;
@@ -589,6 +618,10 @@ rpmhpd_opp_turbo_l1: opp10 {
};
};
};
+
+ apps_bcm_voter: bcm_voter {
+ compatible = "qcom,bcm-voter";
+ };
};
};
--
2.25.1
[PATCH 07/15] ARM: dts: qcom: sdx55: Add SCM node
Add SCM node to enable SCM functionality on SDX55 platform.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 4ca871735025..daf34f24a5d3 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -85,6 +85,12 @@ opp-155520 {
};
};
+ firmware {
+ scm {
+ compatible = "qcom,scm-sdx55", "qcom,scm";
+ };
+ };
+
psci {
compatible = "arm,psci-1.0";
method = "smc";
--
2.25.1
[PATCH 06/15] dt-bindings: firmware: scm: Add compatible for SDX55
Add devicetree compatible for SCM present in SDX55 platform. Cc: Rob Herring Cc: devicet...@vger.kernel.org Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/firmware/qcom,scm.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt index a884955f861e..10281ae6e1b6 100644 --- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt @@ -21,6 +21,7 @@ Required properties: * "qcom,scm-msm8998" * "qcom,scm-sc7180" * "qcom,scm-sdm845" + * "qcom,scm-sdx55" * "qcom,scm-sm8150" * "qcom,scm-sm8250" * "qcom,scm-sm8350" -- 2.25.1
[PATCH 05/15] ARM: dts: qcom: sdx55: Add IMEM and PIL info region
Add a simple-mfd representing IMEM on SDX55 and define the PIL
relocation info region, so that post mortem tools will be able to locate
the loaded remoteproc.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index f2805b526516..4ca871735025 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -379,6 +379,21 @@ tlmm: pinctrl@f10 {
#interrupt-cells = <2>;
};
+ imem@1468f000 {
+ compatible = "simple-mfd";
+ reg = <0x1468f000 0x1000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ranges = <0x0 0x1468f000 0x1000>;
+
+ pil-reloc@94c {
+ compatible = "qcom,pil-reloc-info";
+ reg = <0x94c 0x200>;
+ };
+ };
+
apps_smmu: iommu@1500 {
compatible = "qcom,sdx55-smmu-500", "arm,mmu-500";
reg = <0x1500 0x2>;
--
2.25.1
[PATCH 04/15] ARM: dts: qcom: sdx55: Add modem SMP2P node
Add SMP2P nodes for the SDX55 platform to communicate with the modem.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 31 +++
1 file changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 20cb2c903a91..f2805b526516 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -148,6 +148,37 @@ smem {
hwlocks = <&tcsr_mutex 3>;
};
+ smp2p-mpss {
+ compatible = "qcom,smp2p";
+ qcom,smem = <435>, <428>;
+ interrupts = ;
+ mboxes = <&apcs 14>;
+ qcom,local-pid = <0>;
+ qcom,remote-pid = <1>;
+
+ modem_smp2p_out: master-kernel {
+ qcom,entry-name = "master-kernel";
+ #qcom,smem-state-cells = <1>;
+ };
+
+ modem_smp2p_in: slave-kernel {
+ qcom,entry-name = "slave-kernel";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ ipa_smp2p_out: ipa-ap-to-modem {
+ qcom,entry-name = "ipa";
+ #qcom,smem-state-cells = <1>;
+ };
+
+ ipa_smp2p_in: ipa-modem-to-ap {
+ qcom,entry-name = "ipa";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+ };
+
soc: soc {
#address-cells = <1>;
#size-cells = <1>;
--
2.25.1
[PATCH 03/15] ARM: dts: qcom: sdx55: Add CPUFreq support
Add CPUFreq support to SDX55 platform using the cpufreq-dt driver.
There is no dedicated hardware block available on this platform to
carry on the CPUFreq duties. Hence, it is accomplished using the CPU
clock and regulators tied together by the operating points table.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 8112a5283ce2..20cb2c903a91 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -53,6 +53,35 @@ cpu0: cpu@0 {
compatible = "arm,cortex-a7";
reg = <0x0>;
enable-method = "psci";
+ clocks = <&apcs>;
+ power-domains = <&rpmhpd SDX55_CX>;
+ power-domain-names = "rpmhpd";
+ operating-points-v2 = <&cpu_opp_table>;
+ };
+ };
+
+ cpu_opp_table: cpu-opp-table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-34560 {
+ opp-hz = /bits/ 64 <34560>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ };
+
+ opp-57600 {
+ opp-hz = /bits/ 64 <57600>;
+ required-opps = <&rpmhpd_opp_svs>;
+ };
+
+ opp-109440 {
+ opp-hz = /bits/ 64 <109440>;
+ required-opps = <&rpmhpd_opp_nom>;
+ };
+
+ opp-155520 {
+ opp-hz = /bits/ 64 <155520>;
+ required-opps = <&rpmhpd_opp_turbo>;
};
};
--
2.25.1
[PATCH 02/15] ARM: dts: qcom: sdx55: Add support for APCS block
The APCS block on SDX55 acts as a mailbox controller and also provides
clock output for the Cortex A7 CPU.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 41c90f598359..8112a5283ce2 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -360,6 +360,15 @@ a7pll: clock@17808000 {
#clock-cells = <0>;
};
+ apcs: mailbox@1781 {
+ compatible = "qcom,sdx55-apcs-gcc", "syscon";
+ reg = <0x1781 0x2000>;
+ #mbox-cells = <1>;
+ clocks = <&rpmhcc RPMH_CXO_CLK>, <&a7pll>, <&gcc GPLL0>;
+ clock-names = "ref", "pll", "aux";
+ #clock-cells = <0>;
+ };
+
watchdog@17817000 {
compatible = "qcom,apss-wdt-sdx55", "qcom,kpss-wdt";
reg = <0x17817000 0x1000>;
--
2.25.1
[PATCH 01/15] ARM: dts: qcom: sdx55: Add support for A7 PLL clock
On SDX55 there is a separate A7 PLL which is used to provide high
frequency clock to the Cortex A7 CPU via a MUX.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm/boot/dts/qcom-sdx55.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi
b/arch/arm/boot/dts/qcom-sdx55.dtsi
index e4180bbc4655..41c90f598359 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -352,6 +352,14 @@ intc: interrupt-controller@1780 {
<0x17802000 0x1000>;
};
+ a7pll: clock@17808000 {
+ compatible = "qcom,sdx55-a7pll";
+ reg = <0x17808000 0x1000>;
+ clocks = <&rpmhcc RPMH_CXO_CLK>;
+ clock-names = "bi_tcxo";
+ #clock-cells = <0>;
+ };
+
watchdog@17817000 {
compatible = "qcom,apss-wdt-sdx55", "qcom,kpss-wdt";
reg = <0x17817000 0x1000>;
--
2.25.1
[PATCH 00/15] SDX55 devicetree updates
Hi Bjorn, This series updates the SDX55 devicetree by adding below features: - A7 PLL - APCS mailbox - CPUFreq using clk and regulator - SMP2P - IMEM, PIL - SCM - Interconnect - Telit FN980 TLB board - Thundercomm T55 dev board - Modem remoteproc Except remoteproc, all of the driver patches already merged. Remoteproc patch will be submitted separately. Thanks, Mani Manivannan Sadhasivam (15): ARM: dts: qcom: sdx55: Add support for A7 PLL clock ARM: dts: qcom: sdx55: Add support for APCS block ARM: dts: qcom: sdx55: Add CPUFreq support ARM: dts: qcom: sdx55: Add modem SMP2P node ARM: dts: qcom: sdx55: Add IMEM and PIL info region dt-bindings: firmware: scm: Add compatible for SDX55 ARM: dts: qcom: sdx55: Add SCM node ARM: dts: qcom: sdx55: Add interconnect nodes ARM: dts: qcom: Fix node name for NAND controller node dt-bindings: arm: qcom: Add binding for Telit FN980 TLB board ARM: dts: qcom: sdx55: Add basic devicetree support for Telit FN980 TLB dt-bindings: arm: qcom: Add binding for Thundercomm T55 kit ARM: dts: qcom: sdx55: Add basic devicetree support for Thundercomm T55 dt-bindings: remoteproc: qcom: pas: Add binding for SDX55 ARM: dts: qcom: sdx55: Add Modem remoteproc node .../devicetree/bindings/arm/qcom.yaml | 2 + .../devicetree/bindings/firmware/qcom,scm.txt | 1 + .../bindings/remoteproc/qcom,adsp.txt | 4 + arch/arm/boot/dts/Makefile| 4 +- arch/arm/boot/dts/qcom-ipq4019.dtsi | 2 +- arch/arm/boot/dts/qcom-sdx55-t55.dts | 281 + .../boot/dts/qcom-sdx55-telit-fn980-tlb.dts | 282 ++ arch/arm/boot/dts/qcom-sdx55.dtsi | 166 ++- 8 files changed, 739 insertions(+), 3 deletions(-) create mode 100644 arch/arm/boot/dts/qcom-sdx55-t55.dts create mode 100644 arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dts -- 2.25.1
Re: [PATCH 1/2] [v2] bus: mhi: pci_generic: Introduce Foxconn T99W175 support
On Thu, Apr 08, 2021 at 02:55:24AM -0700, Jarvis Jiang wrote:
> Add support for T99W175 modems, this modem series is based on SDX55
> qcom chip. The modem is mainly based on MBIM protocol for both the
> data and control path.
>
> This patch adds support for below modems:
>
> - T99W175(based on sdx55), Both for eSIM and Non-eSIM
> - DW5930e(based on sdx55), With eSIM, It's also T99W175
> - DW5930e(based on sdx55), Non-eSIM, It's also T99W175
>
> This patch was tested with Ubuntu 20.04 X86_64 PC as host
>
> Signed-off-by: Jarvis Jiang
Applied to mhi-next!
Thanks,
Mani
> ---
> drivers/bus/mhi/pci_generic.c | 47 +++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> index 544853c67e02..c66fb73e47ad 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -269,6 +269,44 @@ static const struct mhi_pci_dev_info
> mhi_quectel_em1xx_info = {
> .dma_data_width = 32
> };
>
> +static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> + MHI_CHANNEL_CONFIG_UL(0, "LOOPBACK", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(1, "LOOPBACK", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(4, "DIAG", 32, 1),
> + MHI_CHANNEL_CONFIG_DL(5, "DIAG", 32, 1),
> + MHI_CHANNEL_CONFIG_UL(12, "MBIM", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(32, "AT", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(33, "AT", 32, 0),
> + MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> + MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> +};
> +
> +static struct mhi_event_config mhi_foxconn_sdx55_events[] = {
> + MHI_EVENT_CONFIG_CTRL(0, 128),
> + MHI_EVENT_CONFIG_DATA(1, 128),
> + MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100),
> + MHI_EVENT_CONFIG_HW_DATA(3, 1024, 101)
> +};
> +
> +static struct mhi_controller_config modem_foxconn_sdx55_config = {
> + .max_channels = 128,
> + .timeout_ms = 2,
> + .num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> + .ch_cfg = mhi_foxconn_sdx55_channels,
> + .num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> + .event_cfg = mhi_foxconn_sdx55_events,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> + .name = "foxconn-sdx55",
> + .fw = "qcom/sdx55m/sbl1.mbn",
> + .edl = "qcom/sdx55m/edl.mbn",
> + .config = &modem_foxconn_sdx55_config,
> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> + .dma_data_width = 32
> +};
> +
> static const struct pci_device_id mhi_pci_id_table[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0306),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx55_info },
> @@ -280,6 +318,15 @@ static const struct pci_device_id mhi_pci_id_table[] = {
> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info },
> + /* T99W175 (sdx55), Both for eSIM and Non-eSIM */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + /* DW5930e (sdx55), With eSIM, It's also T99W175 */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b0),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + /* DW5930e (sdx55), Non-eSIM, It's also T99W175 */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b1),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> { }
> };
> MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
> --
> 2.25.1
>
[PATCH] bus: mhi: pci_generic: Constify mhi_controller_config struct definitions
"mhi_controller_config" struct is not modified inside "mhi_pci_dev_info"
struct. So constify the instances.
Signed-off-by: Manivannan Sadhasivam
---
drivers/bus/mhi/pci_generic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
index 8f715519ba08..7c810f02a2ef 100644
--- a/drivers/bus/mhi/pci_generic.c
+++ b/drivers/bus/mhi/pci_generic.c
@@ -227,7 +227,7 @@ static struct mhi_event_config modem_qcom_v1_mhi_events[] =
{
MHI_EVENT_CONFIG_HW_DATA(3, 2048, 101)
};
-static struct mhi_controller_config modem_qcom_v1_mhiv_config = {
+static const struct mhi_controller_config modem_qcom_v1_mhiv_config = {
.max_channels = 128,
.timeout_ms = 8000,
.num_channels = ARRAY_SIZE(modem_qcom_v1_mhi_channels),
@@ -287,7 +287,7 @@ static struct mhi_event_config mhi_quectel_em1xx_events[] =
{
MHI_EVENT_CONFIG_HW_DATA(3, 1024, 101)
};
-static struct mhi_controller_config modem_quectel_em1xx_config = {
+static const struct mhi_controller_config modem_quectel_em1xx_config = {
.max_channels = 128,
.timeout_ms = 2,
.num_channels = ARRAY_SIZE(mhi_quectel_em1xx_channels),
@@ -324,7 +324,7 @@ static struct mhi_event_config mhi_foxconn_sdx55_events[] =
{
MHI_EVENT_CONFIG_HW_DATA(3, 1024, 101)
};
-static struct mhi_controller_config modem_foxconn_sdx55_config = {
+static const struct mhi_controller_config modem_foxconn_sdx55_config = {
.max_channels = 128,
.timeout_ms = 2,
.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
--
2.25.1
Re: [PATCH 1/2] [v2] bus: mhi: pci_generic: Introduce Foxconn T99W175 support
On Thu, Apr 08, 2021 at 02:55:24AM -0700, Jarvis Jiang wrote:
> Add support for T99W175 modems, this modem series is based on SDX55
> qcom chip. The modem is mainly based on MBIM protocol for both the
> data and control path.
>
> This patch adds support for below modems:
>
> - T99W175(based on sdx55), Both for eSIM and Non-eSIM
> - DW5930e(based on sdx55), With eSIM, It's also T99W175
> - DW5930e(based on sdx55), Non-eSIM, It's also T99W175
>
> This patch was tested with Ubuntu 20.04 X86_64 PC as host
>
The subject should be as below,
[PATCH v2] bus: mhi: pci_generic: Introduce Foxconn T99W175 support
> Signed-off-by: Jarvis Jiang
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
Also you need to add the changelog here.
> drivers/bus/mhi/pci_generic.c | 47 +++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> index 544853c67e02..c66fb73e47ad 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -269,6 +269,44 @@ static const struct mhi_pci_dev_info
> mhi_quectel_em1xx_info = {
> .dma_data_width = 32
> };
>
> +static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> + MHI_CHANNEL_CONFIG_UL(0, "LOOPBACK", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(1, "LOOPBACK", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(4, "DIAG", 32, 1),
> + MHI_CHANNEL_CONFIG_DL(5, "DIAG", 32, 1),
> + MHI_CHANNEL_CONFIG_UL(12, "MBIM", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(32, "AT", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(33, "AT", 32, 0),
> + MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> + MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> +};
> +
> +static struct mhi_event_config mhi_foxconn_sdx55_events[] = {
> + MHI_EVENT_CONFIG_CTRL(0, 128),
> + MHI_EVENT_CONFIG_DATA(1, 128),
> + MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100),
> + MHI_EVENT_CONFIG_HW_DATA(3, 1024, 101)
> +};
> +
> +static struct mhi_controller_config modem_foxconn_sdx55_config = {
> + .max_channels = 128,
> + .timeout_ms = 2,
> + .num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> + .ch_cfg = mhi_foxconn_sdx55_channels,
> + .num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> + .event_cfg = mhi_foxconn_sdx55_events,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> + .name = "foxconn-sdx55",
> + .fw = "qcom/sdx55m/sbl1.mbn",
> + .edl = "qcom/sdx55m/edl.mbn",
> + .config = &modem_foxconn_sdx55_config,
> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> + .dma_data_width = 32
> +};
> +
> static const struct pci_device_id mhi_pci_id_table[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0306),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx55_info },
> @@ -280,6 +318,15 @@ static const struct pci_device_id mhi_pci_id_table[] = {
> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info },
> + /* T99W175 (sdx55), Both for eSIM and Non-eSIM */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + /* DW5930e (sdx55), With eSIM, It's also T99W175 */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b0),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + /* DW5930e (sdx55), Non-eSIM, It's also T99W175 */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b1),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> { }
> };
> MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
> --
> 2.25.1
>
[PATCH v2] bus: mhi: core: Fix shadow declarations
This commit fixes below sparse warnings with W=2 about shadow
declarations:
drivers/bus/mhi/core/main.c: In function ‘parse_xfer_event’:
drivers/bus/mhi/core/main.c:667:17: warning: declaration of ‘flags’ shadows a
previous local [-Wshadow]
667 | unsigned long flags;
| ^
drivers/bus/mhi/core/main.c:565:16: note: shadowed declaration is here
565 | unsigned long flags = 0;
|^
drivers/bus/mhi/core/main.c: In function ‘mhi_process_ctrl_ev_ring’:
drivers/bus/mhi/core/main.c:856:23: warning: declaration of ‘new_state’ shadows
a previous local [-Wshadow]
856 | enum mhi_pm_state new_state;
| ^
drivers/bus/mhi/core/main.c:837:19: note: shadowed declaration is here
837 |enum mhi_state new_state;
| ^
Signed-off-by: Manivannan Sadhasivam
---
Changes in v2:
* Used a separate "mhi_pm_state" variable
drivers/bus/mhi/core/main.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index b0c8afe16e3a..47a8df550fe0 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -664,8 +664,6 @@ static int parse_xfer_event(struct mhi_controller
*mhi_cntrl,
case MHI_EV_CC_OOB:
case MHI_EV_CC_DB_MODE:
{
- unsigned long flags;
-
mhi_chan->db_cfg.db_mode = 1;
read_lock_irqsave(&mhi_cntrl->pm_lock, flags);
if (tre_ring->wp != tre_ring->rp &&
@@ -853,14 +851,14 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
break;
case MHI_STATE_SYS_ERR:
{
- enum mhi_pm_state new_state;
+ enum mhi_pm_state pm_state;
dev_dbg(dev, "System error detected\n");
write_lock_irq(&mhi_cntrl->pm_lock);
- new_state = mhi_tryset_pm_state(mhi_cntrl,
+ pm_state = mhi_tryset_pm_state(mhi_cntrl,
MHI_PM_SYS_ERR_DETECT);
write_unlock_irq(&mhi_cntrl->pm_lock);
- if (new_state == MHI_PM_SYS_ERR_DETECT)
+ if (pm_state == MHI_PM_SYS_ERR_DETECT)
mhi_pm_sys_err_handler(mhi_cntrl);
break;
}
--
2.25.1
Re: [PATCH v3] bus: mhi: core: Sanity check values from remote device before use
On Wed, Mar 10, 2021 at 02:30:55PM -0700, Jeffrey Hugo wrote:
> When parsing the structures in the shared memory, there are values which
> come from the remote device. For example, a transfer completion event
> will have a pointer to the tre in the relevant channel's transfer ring.
> As another example, event ring elements may specify a channel in which
> the event occurred, however the specified channel value may not be valid
> as no channel is defined at that index even though the index may be less
> than the maximum allowed index. Such values should be considered to be
> untrusted, and validated before use. If we blindly use such values, we
> may access invalid data or crash if the values are corrupted.
>
> If validation fails, drop the relevant event.
>
> Signed-off-by: Jeffrey Hugo
Applied to mhi-next!
Thanks,
Mani
> ---
>
> v3: Add the channel validation example to commit text
> v2: Fix subject
>
> drivers/bus/mhi/core/main.c | 81
> +
> 1 file changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index a7811fb..6c0e05d 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -242,6 +242,11 @@ static void mhi_del_ring_element(struct mhi_controller
> *mhi_cntrl,
> smp_wmb();
> }
>
> +static bool is_valid_ring_ptr(struct mhi_ring *ring, dma_addr_t addr)
> +{
> + return addr >= ring->iommu_base && addr < ring->iommu_base + ring->len;
> +}
> +
> int mhi_destroy_device(struct device *dev, void *data)
> {
> struct mhi_chan *ul_chan, *dl_chan;
> @@ -404,7 +409,16 @@ irqreturn_t mhi_irq_handler(int irq_number, void *dev)
> struct mhi_event_ctxt *er_ctxt =
> &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index];
> struct mhi_ring *ev_ring = &mhi_event->ring;
> - void *dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);
> + dma_addr_t ptr = er_ctxt->rp;
> + void *dev_rp;
> +
> + if (!is_valid_ring_ptr(ev_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event ring rp points outside of the event ring\n");
> + return IRQ_HANDLED;
> + }
> +
> + dev_rp = mhi_to_virtual(ev_ring, ptr);
>
> /* Only proceed if event ring has pending events */
> if (ev_ring->rp == dev_rp)
> @@ -560,6 +574,11 @@ static int parse_xfer_event(struct mhi_controller
> *mhi_cntrl,
> struct mhi_buf_info *buf_info;
> u16 xfer_len;
>
> + if (!is_valid_ring_ptr(tre_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event element points outside of the tre
> ring\n");
> + break;
> + }
> /* Get the TRB this event points to */
> ev_tre = mhi_to_virtual(tre_ring, ptr);
>
> @@ -719,6 +738,12 @@ static void mhi_process_cmd_completion(struct
> mhi_controller *mhi_cntrl,
> struct mhi_chan *mhi_chan;
> u32 chan;
>
> + if (!is_valid_ring_ptr(mhi_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event element points outside of the cmd ring\n");
> + return;
> + }
> +
> cmd_pkt = mhi_to_virtual(mhi_ring, ptr);
>
> chan = MHI_TRE_GET_CMD_CHID(cmd_pkt);
> @@ -743,6 +768,7 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
> struct device *dev = &mhi_cntrl->mhi_dev->dev;
> u32 chan;
> int count = 0;
> + dma_addr_t ptr = er_ctxt->rp;
>
> /*
>* This is a quick check to avoid unnecessary event processing
> @@ -752,7 +778,13 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
> if (unlikely(MHI_EVENT_ACCESS_INVALID(mhi_cntrl->pm_state)))
> return -EIO;
>
> - dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);
> + if (!is_valid_ring_ptr(ev_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event ring rp points outside of the event ring\n");
> + return -EIO;
> + }
> +
> + dev_rp = mhi_to_virtual(ev_ring, ptr);
> local_rp = ev_ring->rp;
>
> while (dev_rp != local_rp) {
> @@ -858,6 +890,8 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
>*/
> if (chan < mhi_cntrl->max_chan) {
> mhi_chan = &mhi_cntrl->mhi_chan[chan];
> + if (!mhi_chan->configured)
> + break;
> parse_xfer_event(mhi_cntrl, local_rp, mhi_chan);
> event_quota--;
> }
> @@ -869,7 +903,15 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
>
> mhi_recycle_ev_ring_element(mhi_cntrl, ev_ring);
> local_rp = ev_ring->rp;
> - dev_rp = mhi
[PATCH] bus: mhi: core: Fix shadow declarations
This commit fixes below sparse warnings with W=2 about shadow
declarations:
drivers/bus/mhi/core/main.c: In function ‘parse_xfer_event’:
drivers/bus/mhi/core/main.c:667:17: warning: declaration of ‘flags’ shadows a
previous local [-Wshadow]
667 | unsigned long flags;
| ^
drivers/bus/mhi/core/main.c:565:16: note: shadowed declaration is here
565 | unsigned long flags = 0;
|^
drivers/bus/mhi/core/main.c: In function ‘mhi_process_ctrl_ev_ring’:
drivers/bus/mhi/core/main.c:856:23: warning: declaration of ‘new_state’ shadows
a previous local [-Wshadow]
856 | enum mhi_pm_state new_state;
| ^
drivers/bus/mhi/core/main.c:837:19: note: shadowed declaration is here
837 |enum mhi_state new_state;
| ^
Signed-off-by: Manivannan Sadhasivam
---
drivers/bus/mhi/core/main.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index b0c8afe16e3a..8e28e4a3ab4b 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -664,8 +664,6 @@ static int parse_xfer_event(struct mhi_controller
*mhi_cntrl,
case MHI_EV_CC_OOB:
case MHI_EV_CC_DB_MODE:
{
- unsigned long flags;
-
mhi_chan->db_cfg.db_mode = 1;
read_lock_irqsave(&mhi_cntrl->pm_lock, flags);
if (tre_ring->wp != tre_ring->rp &&
@@ -853,8 +851,6 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
break;
case MHI_STATE_SYS_ERR:
{
- enum mhi_pm_state new_state;
-
dev_dbg(dev, "System error detected\n");
write_lock_irq(&mhi_cntrl->pm_lock);
new_state = mhi_tryset_pm_state(mhi_cntrl,
--
2.25.1
Re: [PATCH v3] bus: mhi: core: Sanity check values from remote device before use
On Wed, Mar 10, 2021 at 02:30:55PM -0700, Jeffrey Hugo wrote:
> When parsing the structures in the shared memory, there are values which
> come from the remote device. For example, a transfer completion event
> will have a pointer to the tre in the relevant channel's transfer ring.
> As another example, event ring elements may specify a channel in which
> the event occurred, however the specified channel value may not be valid
> as no channel is defined at that index even though the index may be less
> than the maximum allowed index. Such values should be considered to be
> untrusted, and validated before use. If we blindly use such values, we
> may access invalid data or crash if the values are corrupted.
>
> If validation fails, drop the relevant event.
>
> Signed-off-by: Jeffrey Hugo
Looks good to me, but I need an ACK from Hemant/Bhaumik.
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
>
> v3: Add the channel validation example to commit text
> v2: Fix subject
>
> drivers/bus/mhi/core/main.c | 81
> +
> 1 file changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index a7811fb..6c0e05d 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -242,6 +242,11 @@ static void mhi_del_ring_element(struct mhi_controller
> *mhi_cntrl,
> smp_wmb();
> }
>
> +static bool is_valid_ring_ptr(struct mhi_ring *ring, dma_addr_t addr)
> +{
> + return addr >= ring->iommu_base && addr < ring->iommu_base + ring->len;
> +}
> +
> int mhi_destroy_device(struct device *dev, void *data)
> {
> struct mhi_chan *ul_chan, *dl_chan;
> @@ -404,7 +409,16 @@ irqreturn_t mhi_irq_handler(int irq_number, void *dev)
> struct mhi_event_ctxt *er_ctxt =
> &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index];
> struct mhi_ring *ev_ring = &mhi_event->ring;
> - void *dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);
> + dma_addr_t ptr = er_ctxt->rp;
> + void *dev_rp;
> +
> + if (!is_valid_ring_ptr(ev_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event ring rp points outside of the event ring\n");
> + return IRQ_HANDLED;
> + }
> +
> + dev_rp = mhi_to_virtual(ev_ring, ptr);
>
> /* Only proceed if event ring has pending events */
> if (ev_ring->rp == dev_rp)
> @@ -560,6 +574,11 @@ static int parse_xfer_event(struct mhi_controller
> *mhi_cntrl,
> struct mhi_buf_info *buf_info;
> u16 xfer_len;
>
> + if (!is_valid_ring_ptr(tre_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event element points outside of the tre
> ring\n");
> + break;
> + }
> /* Get the TRB this event points to */
> ev_tre = mhi_to_virtual(tre_ring, ptr);
>
> @@ -719,6 +738,12 @@ static void mhi_process_cmd_completion(struct
> mhi_controller *mhi_cntrl,
> struct mhi_chan *mhi_chan;
> u32 chan;
>
> + if (!is_valid_ring_ptr(mhi_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event element points outside of the cmd ring\n");
> + return;
> + }
> +
> cmd_pkt = mhi_to_virtual(mhi_ring, ptr);
>
> chan = MHI_TRE_GET_CMD_CHID(cmd_pkt);
> @@ -743,6 +768,7 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
> struct device *dev = &mhi_cntrl->mhi_dev->dev;
> u32 chan;
> int count = 0;
> + dma_addr_t ptr = er_ctxt->rp;
>
> /*
>* This is a quick check to avoid unnecessary event processing
> @@ -752,7 +778,13 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
> if (unlikely(MHI_EVENT_ACCESS_INVALID(mhi_cntrl->pm_state)))
> return -EIO;
>
> - dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);
> + if (!is_valid_ring_ptr(ev_ring, ptr)) {
> + dev_err(&mhi_cntrl->mhi_dev->dev,
> + "Event ring rp points outside of the event ring\n");
> + return -EIO;
> + }
> +
> + dev_rp = mhi_to_virtual(ev_ring, ptr);
> local_rp = ev_ring->rp;
>
> while (dev_rp != local_rp) {
> @@ -858,6 +890,8 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
> *mhi_cntrl,
>*/
> if (chan &
Re: your mail
On Thu, Apr 01, 2021 at 02:16:09PM -0700, Bhaumik Bhatt wrote: > Subject: [PATCH v8 0/9] Updates to MHI channel handling > Subject is present in the body ;) > MHI specification shows a state machine with support for STOP channel command > and the validity of certain state transitions. MHI host currently does not > provide any mechanism to stop a channel and restart it without resetting it. > There are also times when the device moves on to a different execution > environment while client drivers on the host are unaware of it and still > attempt to reset the channels facing unnecessary timeouts. > > This series addresses the above areas to provide support for stopping an MHI > channel, resuming it back, improved documentation and improving upon channel > state machine handling in general. > > This set of patches was tested on arm64 and x86_64 architecture. > Series applied to mhi-next! Thanks, Mani > v8: > -Split the state machine improvements patch to three patches as per review > > v7: > -Tested on x86_64 architecture > -Drop the patch "Do not clear channel context more than once" as issue is > fixed > differently using "bus: mhi: core: Fix double dma free()" > -Update the commit text to better reflect changes on state machine > improvements > > v6: > -Dropped the patch which introduced start/stop transfer APIs for lack of users > -Updated error handling and debug prints on channel handling improvements > patch > -Improved commit text to better explain certain patches based on review > comments > -Removed references to new APIs from the documentation improvement patch > > v5: > -Added reviewed-by tags from Hemant I missed earlier > -Added patch to prevent kernel warnings on clearing channel context twice > > v4: > -Updated commit text/descriptions and addressed checkpatch checks > -Added context validity check before starting/stopping channels from new API > -Added patch to clear channel context configuration after reset/unprepare > > v3: > -Updated documentation for channel transfer APIs to highlight differences > -Create separate patch for "allowing channel to be disabled from stopped > state" > > v2: > -Renamed the newly introduced APIs to mhi_start_transfer() / > mhi_stop_transfer() > -Added improved documentation to avoid confusion with the new APIs > -Removed the __ prefix from mhi_unprepare_channel() API for consistency. > > Bhaumik Bhatt (9): > bus: mhi: core: Allow sending the STOP channel command > bus: mhi: core: Clear context for stopped channels from remove() > bus: mhi: core: Improvements to the channel handling state machine > bus: mhi: core: Update debug messages to use client device > bus: mhi: core: Hold device wake for channel update commands > bus: mhi: core: Clear configuration from channel context during reset > bus: mhi: core: Check channel execution environment before issuing > reset > bus: mhi: core: Remove __ prefix for MHI channel unprepare function > bus: mhi: Improve documentation on channel transfer setup APIs > > drivers/bus/mhi/core/init.c | 22 - > drivers/bus/mhi/core/internal.h | 12 +++ > drivers/bus/mhi/core/main.c | 190 > > include/linux/mhi.h | 18 +++- > 4 files changed, 162 insertions(+), 80 deletions(-) > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
Re: [PATCH] bus: mhi: pci_generic: Add SDX65 based modem support
On Fri, Apr 02, 2021 at 02:33:19PM -0700, Bhaumik Bhatt wrote:
> Add generic info for SDX65 based modems.
>
> Signed-off-by: Bhaumik Bhatt
Applied to mhi-next!
Thanks,
Mani
> ---
> This patch was tested on SDX65 hardware with Ubuntu X86_64 PC as host.
>
> drivers/bus/mhi/pci_generic.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> index 5cf44bc..92a1b18 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -204,6 +204,15 @@ static struct mhi_controller_config
> modem_qcom_v1_mhiv_config = {
> .event_cfg = modem_qcom_v1_mhi_events,
> };
>
> +static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = {
> + .name = "qcom-sdx65m",
> + .fw = "qcom/sdx65m/xbl.elf",
> + .edl = "qcom/sdx65m/edl.mbn",
> + .config = &modem_qcom_v1_mhiv_config,
> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> + .dma_data_width = 32
> +};
> +
> static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = {
> .name = "qcom-sdx55m",
> .fw = "qcom/sdx55m/sbl1.mbn",
> @@ -261,6 +270,8 @@ static const struct mhi_pci_dev_info
> mhi_quectel_em1xx_info = {
> };
>
> static const struct pci_device_id mhi_pci_id_table[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308),
> + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0306),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx55_info },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0304),
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes
On Thu, Apr 01, 2021 at 02:41:49PM -0700, Bhaumik Bhatt wrote:
> Some controllers can choose to skip preparation for power up.
> In that case, device context is initialized based on the pre_init
> flag not being set during mhi_prepare_for_power_up(). There is no
> reason MHI host driver should maintain and provide controllers
> with two separate paths for preparing MHI.
>
> Going forward, all controllers will be required to call the
> mhi_prepare_for_power_up() API followed by their choice of sync
> or async power up. This allows MHI host driver to get rid of the
> pre_init flag and sets up a common way for all controllers to use
> MHI. This also helps controllers fail early on during preparation
> phase in some failure cases.
>
> Signed-off-by: Bhaumik Bhatt
Applied to mhi-next!
Thanks,
Mani
> ---
> This patch was tested on arm64 architecture.
>
> drivers/bus/mhi/core/init.c | 3 ---
> drivers/bus/mhi/core/pm.c | 20
> include/linux/mhi.h | 2 --
> 3 files changed, 25 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index d1d9b0d..1f61352 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -1080,8 +1080,6 @@ int mhi_prepare_for_power_up(struct mhi_controller
> *mhi_cntrl)
> mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
> }
>
> - mhi_cntrl->pre_init = true;
> -
> mutex_unlock(&mhi_cntrl->pm_mutex);
>
> return 0;
> @@ -1112,7 +1110,6 @@ void mhi_unprepare_after_power_down(struct
> mhi_controller *mhi_cntrl)
> }
>
> mhi_deinit_dev_ctxt(mhi_cntrl);
> - mhi_cntrl->pre_init = false;
> }
> EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
>
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e4aff77..b23eec5 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -1062,13 +1062,6 @@ int mhi_async_power_up(struct mhi_controller
> *mhi_cntrl)
> mutex_lock(&mhi_cntrl->pm_mutex);
> mhi_cntrl->pm_state = MHI_PM_DISABLE;
>
> - if (!mhi_cntrl->pre_init) {
> - /* Setup device context */
> - ret = mhi_init_dev_ctxt(mhi_cntrl);
> - if (ret)
> - goto error_dev_ctxt;
> - }
> -
> ret = mhi_init_irq_setup(mhi_cntrl);
> if (ret)
> goto error_setup_irq;
> @@ -1150,10 +1143,6 @@ int mhi_async_power_up(struct mhi_controller
> *mhi_cntrl)
> mhi_deinit_free_irq(mhi_cntrl);
>
> error_setup_irq:
> - if (!mhi_cntrl->pre_init)
> - mhi_deinit_dev_ctxt(mhi_cntrl);
> -
> -error_dev_ctxt:
> mhi_cntrl->pm_state = MHI_PM_DISABLE;
> mutex_unlock(&mhi_cntrl->pm_mutex);
>
> @@ -1203,15 +1192,6 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl,
> bool graceful)
> flush_work(&mhi_cntrl->st_worker);
>
> free_irq(mhi_cntrl->irq[0], mhi_cntrl);
> -
> - if (!mhi_cntrl->pre_init) {
> - /* Free all allocated resources */
> - if (mhi_cntrl->fbc_image) {
> - mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
> - mhi_cntrl->fbc_image = NULL;
> - }
> - mhi_deinit_dev_ctxt(mhi_cntrl);
> - }
> }
> EXPORT_SYMBOL_GPL(mhi_power_down);
>
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index b16afd3..c9b36a3 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -354,7 +354,6 @@ struct mhi_controller_config {
> * @index: Index of the MHI controller instance
> * @bounce_buf: Use of bounce buffer
> * @fbc_download: MHI host needs to do complete image transfer (optional)
> - * @pre_init: MHI host needs to do pre-initialization before power up
> * @wake_set: Device wakeup set flag
> * @irq_flags: irq flags passed to request_irq (optional)
> *
> @@ -447,7 +446,6 @@ struct mhi_controller {
> int index;
> bool bounce_buf;
> bool fbc_download;
> - bool pre_init;
> bool wake_set;
> unsigned long irq_flags;
> };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH] bus: mhi: pci_generic: Introduce Foxconn T99W175 support
On Tue, Apr 06, 2021 at 07:50:29PM -0700, Jarvis Jiang wrote:
> Add support for T99W175 modems, this modem series is based on SDX55
> qcom chip. The modem is mainly based on MBIM protocol for both the
> data and control path.
>
List the modems whose support is being added.
> This patch was tested with Ubuntu 20.04 X86_64 PC as host
>
> Signed-off-by: Jarvis Jiang
> ---
> drivers/bus/mhi/pci_generic.c | 58 +++
> 1 file changed, 58 insertions(+)
>
> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> index 5cf44bcfe040..3e396c65a758 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -260,6 +260,52 @@ static const struct mhi_pci_dev_info
> mhi_quectel_em1xx_info = {
> .dma_data_width = 32
> };
>
> +static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> + MHI_CHANNEL_CONFIG_UL(0, "LOOPBACK", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(1, "LOOPBACK", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(4, "DIAG", 32, 1),
> + MHI_CHANNEL_CONFIG_DL(5, "DIAG", 32, 1),
> + MHI_CHANNEL_CONFIG_UL(12, "MBIM", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(14, "QMI", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(15, "QMI", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(16, "QMI1", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(17, "QMI1", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(18, "IP_CTRL", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(19, "IP_CTRL", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(20, "IPCR", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(21, "IPCR", 32, 0),
> + MHI_CHANNEL_CONFIG_UL(32, "AT", 32, 0),
> + MHI_CHANNEL_CONFIG_DL(33, "AT", 32, 0),
> + MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> + MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> +};
> +
> +static struct mhi_event_config mhi_foxconn_sdx55_events[] = {
> + MHI_EVENT_CONFIG_CTRL(0, 128),
> + MHI_EVENT_CONFIG_DATA(1, 128),
> + MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100),
> + MHI_EVENT_CONFIG_HW_DATA(3, 1024, 101)
> +};
> +
> +static struct mhi_controller_config modem_foxconn_sdx55_config = {
> + .max_channels = 128,
> + .timeout_ms = 2,
> + .num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> + .ch_cfg = mhi_foxconn_sdx55_channels,
> + .num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> + .event_cfg = mhi_foxconn_sdx55_events,
> +};
> +
> +static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
> + .name = "foxconn-sdx55",
> + .fw = "qcom/sdx55m/sbl1.mbn",
> + .edl = "qcom/sdx55m/edl.mbn",
> + .config = &modem_foxconn_sdx55_config,
> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> + .dma_data_width = 32
> +};
> +
> static const struct pci_device_id mhi_pci_id_table[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0306),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx55_info },
> @@ -269,6 +315,18 @@ static const struct pci_device_id mhi_pci_id_table[] = {
> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
> { PCI_DEVICE(0x1eac, 0x1002), /* EM160R-GL (sdx24) */
> .driver_data = (kernel_ulong_t) &mhi_quectel_em1xx_info },
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0ab), /* T99W175 (sdx55) */
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b2), /* T99W175 (sdx55) */
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b3), /* T99W175 (sdx55) */
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
Please add a comment about these devices as you did below. Using T99W175 (sdx55)
for all is not sufficient.
Thanks,
Mani
> + /* DW5930e (sdx55), With eSIM, It's also T99W175 */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b0),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> + /* DW5930e (sdx55), Non-eSIM, It's also T99W175 */
> + { PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0b1),
> + .driver_data = (kernel_ulong_t) &mhi_foxconn_sdx55_info },
> { }
> };
> MODULE_DEVICE_TABLE(pci, mhi_pci_id_table);
> --
> 2.25.1
>
Re: [PATCH v8 5/9] bus: mhi: core: Hold device wake for channel update commands
On Thu, Apr 01, 2021 at 02:16:14PM -0700, Bhaumik Bhatt wrote:
> MHI host can fail early if device is in a bad state by attempting
> to assert device wake and holding the runtime PM vote before
> sending a channel update command instead of performing a wake
> toggle and waiting for a timeout if the send were to fail. This
> can help improve the design and enable shorter wait periods for
> device to respond as votes are already held.
>
> Signed-off-by: Bhaumik Bhatt
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/bus/mhi/core/main.c | 19 ++-
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 94fdbf7..989a2a8 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -1278,16 +1278,18 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> return -EINVAL;
> }
>
> - mhi_cntrl->wake_toggle(mhi_cntrl);
> + /* bring host and device out of suspended states */
> + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev);
> + if (ret)
> + return ret;
> mhi_cntrl->runtime_get(mhi_cntrl);
> - mhi_cntrl->runtime_put(mhi_cntrl);
>
> reinit_completion(&mhi_chan->completion);
> ret = mhi_send_cmd(mhi_cntrl, mhi_chan, cmd);
> if (ret) {
> dev_err(dev, "%d: Failed to send %s channel command\n",
> mhi_chan->chan, TO_CH_STATE_TYPE_STR(to_state));
> - return ret;
> + goto exit_channel_update;
> }
>
> ret = wait_for_completion_timeout(&mhi_chan->completion,
> @@ -1296,9 +1298,12 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> dev_err(dev,
> "%d: Failed to receive %s channel command completion\n",
> mhi_chan->chan, TO_CH_STATE_TYPE_STR(to_state));
> - return -EIO;
> + ret = -EIO;
> + goto exit_channel_update;
> }
>
> + ret = 0;
> +
> if (to_state != MHI_CH_STATE_TYPE_RESET) {
> write_lock_irq(&mhi_chan->lock);
> mhi_chan->ch_state = (to_state == MHI_CH_STATE_TYPE_START) ?
> @@ -1309,7 +1314,11 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> dev_dbg(dev, "%d: Channel state change to %s successful\n",
> mhi_chan->chan, TO_CH_STATE_TYPE_STR(to_state));
>
> - return 0;
> +exit_channel_update:
> + mhi_cntrl->runtime_put(mhi_cntrl);
> + mhi_device_put(mhi_cntrl->mhi_dev);
> +
> + return ret;
> }
>
> static void __mhi_unprepare_channel(struct mhi_controller *mhi_cntrl,
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH v8 4/9] bus: mhi: core: Update debug messages to use client device
On Thu, Apr 01, 2021 at 02:16:13PM -0700, Bhaumik Bhatt wrote:
> Debug messages dealing with client devices use the generic MHI
> controller or parent device along with a channel number. It would
> be better to instead use the client device directly and enable
> better log messages for channel updates.
>
> Suggested-by: Manivannan Sadhasivam
> Signed-off-by: Bhaumik Bhatt
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/bus/mhi/core/main.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 710ca0f..94fdbf7 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -1238,7 +1238,7 @@ static int mhi_update_channel_state(struct
> mhi_controller *mhi_cntrl,
> struct mhi_chan *mhi_chan,
> enum mhi_ch_state_type to_state)
> {
> - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + struct device *dev = &mhi_chan->mhi_dev->dev;
> enum mhi_cmd_type cmd = MHI_CMD_NOP;
> int ret;
>
> @@ -1316,7 +1316,7 @@ static void __mhi_unprepare_channel(struct
> mhi_controller *mhi_cntrl,
> struct mhi_chan *mhi_chan)
> {
> int ret;
> - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + struct device *dev = &mhi_chan->mhi_dev->dev;
>
> mutex_lock(&mhi_chan->mutex);
>
> @@ -1340,13 +1340,11 @@ int mhi_prepare_channel(struct mhi_controller
> *mhi_cntrl,
> struct mhi_chan *mhi_chan)
> {
> int ret = 0;
> - struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + struct device *dev = &mhi_chan->mhi_dev->dev;
>
> if (!(BIT(mhi_cntrl->ee) & mhi_chan->ee_mask)) {
> - dev_err(dev,
> - "Current EE: %s Required EE Mask: 0x%x for chan: %s\n",
> - TO_MHI_EXEC_STR(mhi_cntrl->ee), mhi_chan->ee_mask,
> - mhi_chan->name);
> + dev_err(dev, "Current EE: %s Required EE Mask: 0x%x\n",
> + TO_MHI_EXEC_STR(mhi_cntrl->ee), mhi_chan->ee_mask);
> return -ENOTCONN;
> }
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH v8 3/9] bus: mhi: core: Improvements to the channel handling state machine
On Thu, Apr 01, 2021 at 02:16:12PM -0700, Bhaumik Bhatt wrote:
> Improve the channel handling state machine such that all commands
> go through a common function and a validation process to ensure
> that the state machine is not violated in any way and adheres to
> the MHI specification. Using this common function allows MHI to
> eliminate some unnecessary debug messages and code duplication.
>
> Signed-off-by: Bhaumik Bhatt
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/bus/mhi/core/init.c | 6 ++
> drivers/bus/mhi/core/internal.h | 12
> drivers/bus/mhi/core/main.c | 151
> ++--
> 3 files changed, 100 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index f8ba954..9c2ed92 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -56,6 +56,12 @@ const char * const mhi_state_str[MHI_STATE_MAX] = {
> [MHI_STATE_SYS_ERR] = "SYS ERROR",
> };
>
> +const char * const mhi_ch_state_type_str[MHI_CH_STATE_TYPE_MAX] = {
> + [MHI_CH_STATE_TYPE_RESET] = "RESET",
> + [MHI_CH_STATE_TYPE_STOP] = "STOP",
> + [MHI_CH_STATE_TYPE_START] = "START",
> +};
> +
> static const char * const mhi_pm_state_str[] = {
> [MHI_PM_STATE_DISABLE] = "DISABLE",
> [MHI_PM_STATE_POR] = "POWER ON RESET",
> diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
> index e690f15..5b9ea66 100644
> --- a/drivers/bus/mhi/core/internal.h
> +++ b/drivers/bus/mhi/core/internal.h
> @@ -369,6 +369,18 @@ enum mhi_ch_state {
> MHI_CH_STATE_ERROR = 0x5,
> };
>
> +enum mhi_ch_state_type {
> + MHI_CH_STATE_TYPE_RESET,
> + MHI_CH_STATE_TYPE_STOP,
> + MHI_CH_STATE_TYPE_START,
> + MHI_CH_STATE_TYPE_MAX,
> +};
> +
> +extern const char * const mhi_ch_state_type_str[MHI_CH_STATE_TYPE_MAX];
> +#define TO_CH_STATE_TYPE_STR(state) (((state) >= MHI_CH_STATE_TYPE_MAX) ? \
> + "INVALID_STATE" : \
> + mhi_ch_state_type_str[(state)])
> +
> #define MHI_INVALID_BRSTMODE(mode) (mode != MHI_DB_BRST_DISABLE && \
> mode != MHI_DB_BRST_ENABLE)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 8c510f1..710ca0f 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -1234,56 +1234,105 @@ int mhi_send_cmd(struct mhi_controller *mhi_cntrl,
> return 0;
> }
>
> -static void __mhi_unprepare_channel(struct mhi_controller *mhi_cntrl,
> - struct mhi_chan *mhi_chan)
> +static int mhi_update_channel_state(struct mhi_controller *mhi_cntrl,
> + struct mhi_chan *mhi_chan,
> + enum mhi_ch_state_type to_state)
> {
> - int ret;
> struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + enum mhi_cmd_type cmd = MHI_CMD_NOP;
> + int ret;
>
> - dev_dbg(dev, "Entered: unprepare channel:%d\n", mhi_chan->chan);
> -
> - /* no more processing events for this channel */
> - mutex_lock(&mhi_chan->mutex);
> - write_lock_irq(&mhi_chan->lock);
> - if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED &&
> - mhi_chan->ch_state != MHI_CH_STATE_SUSPENDED) {
> + dev_dbg(dev, "%d: Updating channel state to: %s\n", mhi_chan->chan,
> + TO_CH_STATE_TYPE_STR(to_state));
> +
> + switch (to_state) {
> + case MHI_CH_STATE_TYPE_RESET:
> + write_lock_irq(&mhi_chan->lock);
> + if (mhi_chan->ch_state != MHI_CH_STATE_STOP &&
> + mhi_chan->ch_state != MHI_CH_STATE_ENABLED &&
> + mhi_chan->ch_state != MHI_CH_STATE_SUSPENDED) {
> + write_unlock_irq(&mhi_chan->lock);
> + return -EINVAL;
> + }
> + mhi_chan->ch_state = MHI_CH_STATE_DISABLED;
> write_unlock_irq(&mhi_chan->lock);
> - mutex_unlock(&mhi_chan->mutex);
> - return;
> - }
>
> - mhi_chan->ch_state = MHI_CH_STATE_DISABLED;
> - write_unlock_irq(&mhi_chan->lock);
> + cmd = MHI_CMD_RESET_CHAN;
> + break;
> + case MHI_CH_STATE_TYPE_STOP:
> + if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED)
> + return -EINVAL;
>
> - reinit_completion(&mhi_chan->comp
[PATCH v2] docs: driver-model: Update the documentation for device class
The current documentation about the device class is out of date such
that it refers to non-existent APIs and structures. This commit updates
them to the current device class APIs and structures, removes wordings
that no longer valid while trying to keep the original content intact.
Signed-off-by: Manivannan Sadhasivam
---
Changes in v2:
* Fixed CLASS_ATTR_RW as spotted by Greg
.../driver-api/driver-model/class.rst | 144 --
1 file changed, 66 insertions(+), 78 deletions(-)
diff --git a/Documentation/driver-api/driver-model/class.rst
b/Documentation/driver-api/driver-model/class.rst
index fff55b80e86a..87441f4ac4ac 100644
--- a/Documentation/driver-api/driver-model/class.rst
+++ b/Documentation/driver-api/driver-model/class.rst
@@ -5,12 +5,7 @@ Device Classes
Introduction
A device class describes a type of device, like an audio or network
-device. The following device classes have been identified:
-
-
-
-
-Each device class defines a set of semantics and a programming interface
+device. It defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
@@ -18,23 +13,27 @@ a particular bus.
Device classes are agnostic with respect to what bus a device resides
on.
-
Programming Interface
~
The device class structure looks like::
+ struct class {
+const char *name;
+struct module *owner;
- typedef int (*devclass_add)(struct device *);
- typedef void (*devclass_remove)(struct device *);
+const struct attribute_group**class_groups;
+const struct attribute_group**dev_groups;
+struct kobject *dev_kobj;
+...
+ };
See the kerneldoc for the struct class.
A typical device class definition would look like::
- struct device_class input_devclass = {
-.name = "input",
-.add_device= input_add_device,
- .remove_device = input_remove_device,
+ struct class input_class = {
+.name = "input",
+.dev_release= input_dev_release,
};
Each device class structure should be exported in a header file so it
@@ -42,101 +41,84 @@ can be used by drivers, extensions and interfaces.
Device classes are registered and unregistered with the core using::
- int devclass_register(struct device_class * cls);
- void devclass_unregister(struct device_class * cls);
-
+ int class_register(struct class *class);
+ void class_unregister(struct class *class);
Devices
~~~
-As devices are bound to drivers, they are added to the device class
-that the driver belongs to. Before the driver model core, this would
-typically happen during the driver's probe() callback, once the device
-has been initialized. It now happens after the probe() callback
-finishes from the core.
-
-The device is enumerated in the class. Each time a device is added to
-the class, the class's devnum field is incremented and assigned to the
-device. The field is never decremented, so if the device is removed
-from the class and re-added, it will receive a different enumerated
-value.
-
-The class is allowed to create a class-specific structure for the
-device and store it in the device's class_data pointer.
-
-There is no list of devices in the device class. Each driver has a
-list of devices that it supports. The device class has a list of
-drivers of that particular class. To access all of the devices in the
-class, iterate over the device lists of each driver in the class.
+When a device is added, it is also added to the 'klist_devices' inside
+the 'subsys_private' struct of the class. Later, the devices belonging
+to the class are accessed using::
+ class_dev_iter_next()
+ class_find_device()
+ class_find_device_by_name()
-Device Drivers
-~~
-Device drivers are added to device classes when they are registered
-with the core. A driver specifies the class it belongs to by setting
-the struct device_driver::devclass field.
+It is also possible to access the devices of a class in a platform
+dependent way using::
+ class_find_device_by_of_node()
+ class_find_device_by_acpi_dev()
sysfs directory structure
There is a top-level sysfs directory named 'class'.
-Each class gets a directory in the class directory, along with two
-default subdirectories::
+Each class gets a directory in the top-level class directory::
-class/
-`-- input
-|-- devices
-`-- drivers
+ class/
+ |-- input
+ |-- block
+ |-- drm
+ |-- nvme
-
-Drivers registered with the class get a symlink in the drivers/ directory
-that points to the driver's directory (under its bus directory)::
-
- class/
- `-- input
Re: [PATCH] bus: mhi: pci_generic: Add SDX65 based modem support
On Fri, Apr 02, 2021 at 02:33:19PM -0700, Bhaumik Bhatt wrote:
> Add generic info for SDX65 based modems.
>
> Signed-off-by: Bhaumik Bhatt
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> This patch was tested on SDX65 hardware with Ubuntu X86_64 PC as host.
>
> drivers/bus/mhi/pci_generic.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> index 5cf44bc..92a1b18 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -204,6 +204,15 @@ static struct mhi_controller_config
> modem_qcom_v1_mhiv_config = {
> .event_cfg = modem_qcom_v1_mhi_events,
> };
>
> +static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = {
> + .name = "qcom-sdx65m",
> + .fw = "qcom/sdx65m/xbl.elf",
> + .edl = "qcom/sdx65m/edl.mbn",
> + .config = &modem_qcom_v1_mhiv_config,
> + .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> + .dma_data_width = 32
> +};
> +
> static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = {
> .name = "qcom-sdx55m",
> .fw = "qcom/sdx55m/sbl1.mbn",
> @@ -261,6 +270,8 @@ static const struct mhi_pci_dev_info
> mhi_quectel_em1xx_info = {
> };
>
> static const struct pci_device_id mhi_pci_id_table[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0308),
> + .driver_data = (kernel_ulong_t) &mhi_qcom_sdx65_info },
This should come last. I'll fix it while applying.
Thanks,
Mani
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0306),
> .driver_data = (kernel_ulong_t) &mhi_qcom_sdx55_info },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x0304),
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes
On Thu, Apr 01, 2021 at 02:41:49PM -0700, Bhaumik Bhatt wrote:
> Some controllers can choose to skip preparation for power up.
> In that case, device context is initialized based on the pre_init
> flag not being set during mhi_prepare_for_power_up(). There is no
> reason MHI host driver should maintain and provide controllers
> with two separate paths for preparing MHI.
>
> Going forward, all controllers will be required to call the
> mhi_prepare_for_power_up() API followed by their choice of sync
> or async power up. This allows MHI host driver to get rid of the
> pre_init flag and sets up a common way for all controllers to use
> MHI. This also helps controllers fail early on during preparation
> phase in some failure cases.
>
> Signed-off-by: Bhaumik Bhatt
I hope Jeff is also okay with this patch for AIC100.
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> This patch was tested on arm64 architecture.
>
> drivers/bus/mhi/core/init.c | 3 ---
> drivers/bus/mhi/core/pm.c | 20
> include/linux/mhi.h | 2 --
> 3 files changed, 25 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index d1d9b0d..1f61352 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -1080,8 +1080,6 @@ int mhi_prepare_for_power_up(struct mhi_controller
> *mhi_cntrl)
> mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
> }
>
> - mhi_cntrl->pre_init = true;
> -
> mutex_unlock(&mhi_cntrl->pm_mutex);
>
> return 0;
> @@ -1112,7 +1110,6 @@ void mhi_unprepare_after_power_down(struct
> mhi_controller *mhi_cntrl)
> }
>
> mhi_deinit_dev_ctxt(mhi_cntrl);
> - mhi_cntrl->pre_init = false;
> }
> EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
>
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e4aff77..b23eec5 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -1062,13 +1062,6 @@ int mhi_async_power_up(struct mhi_controller
> *mhi_cntrl)
> mutex_lock(&mhi_cntrl->pm_mutex);
> mhi_cntrl->pm_state = MHI_PM_DISABLE;
>
> - if (!mhi_cntrl->pre_init) {
> - /* Setup device context */
> - ret = mhi_init_dev_ctxt(mhi_cntrl);
> - if (ret)
> - goto error_dev_ctxt;
> - }
> -
> ret = mhi_init_irq_setup(mhi_cntrl);
> if (ret)
> goto error_setup_irq;
> @@ -1150,10 +1143,6 @@ int mhi_async_power_up(struct mhi_controller
> *mhi_cntrl)
> mhi_deinit_free_irq(mhi_cntrl);
>
> error_setup_irq:
> - if (!mhi_cntrl->pre_init)
> - mhi_deinit_dev_ctxt(mhi_cntrl);
> -
> -error_dev_ctxt:
> mhi_cntrl->pm_state = MHI_PM_DISABLE;
> mutex_unlock(&mhi_cntrl->pm_mutex);
>
> @@ -1203,15 +1192,6 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl,
> bool graceful)
> flush_work(&mhi_cntrl->st_worker);
>
> free_irq(mhi_cntrl->irq[0], mhi_cntrl);
> -
> - if (!mhi_cntrl->pre_init) {
> - /* Free all allocated resources */
> - if (mhi_cntrl->fbc_image) {
> - mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
> - mhi_cntrl->fbc_image = NULL;
> - }
> - mhi_deinit_dev_ctxt(mhi_cntrl);
> - }
> }
> EXPORT_SYMBOL_GPL(mhi_power_down);
>
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index b16afd3..c9b36a3 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -354,7 +354,6 @@ struct mhi_controller_config {
> * @index: Index of the MHI controller instance
> * @bounce_buf: Use of bounce buffer
> * @fbc_download: MHI host needs to do complete image transfer (optional)
> - * @pre_init: MHI host needs to do pre-initialization before power up
> * @wake_set: Device wakeup set flag
> * @irq_flags: irq flags passed to request_irq (optional)
> *
> @@ -447,7 +446,6 @@ struct mhi_controller {
> int index;
> bool bounce_buf;
> bool fbc_download;
> - bool pre_init;
> bool wake_set;
> unsigned long irq_flags;
> };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
[PATCH] mtd: rawnand: qcom: Use dma_mapping_error() for error check
dma_mapping_error() should be used for checking the error value of dma_map_resource() API. Signed-off-by: Manivannan Sadhasivam --- drivers/mtd/nand/raw/qcom_nandc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c index fe74cf3aece5..07b3c156a802 100644 --- a/drivers/mtd/nand/raw/qcom_nandc.c +++ b/drivers/mtd/nand/raw/qcom_nandc.c @@ -2990,7 +2990,7 @@ static int qcom_nandc_probe(struct platform_device *pdev) nandc->base_dma = dma_map_resource(dev, res->start, resource_size(res), DMA_BIDIRECTIONAL, 0); - if (!nandc->base_dma) + if (dma_mapping_error(dev, nandc->base_dma)) return -ENXIO; ret = qcom_nandc_alloc(nandc); -- 2.25.1
Re: [PATCH] docs: driver-model: Update the documentation for device class
On Sat, Apr 03, 2021 at 03:04:42PM +0200, Greg KH wrote: > On Sat, Apr 03, 2021 at 05:30:50PM +0530, Manivannan Sadhasivam wrote: > > The current documentation about the device class is out of date such > > that it refers to non-existent APIs and structures. This commit updates > > them to the current device class APIs and structures, removes wordings > > that no longer valid while trying to keep the original content intact. > > Thanks for working on this! > > One thing that instantly jumped out at me: > > > -Class drivers can export attributes using the DEVCLASS_ATTR macro that > > works > > -similarly to the DEVICE_ATTR macro for devices. For example, a definition > > +Class drivers can export attributes using the CLASS_ATTR_* macros that > > works > > +similarly to the DEVICE_ATTR_* macros for devices. For example, a > > definition > > like this:: > > > > - static DEVCLASS_ATTR(debug,0644,show_debug,store_debug); > > + static CLASS_ATTR_RW(debug, 0644, show_debug, store_debug); > > CLASS_ATTR_RW(debug); > is the correct way to write the above, what you added there will not > build. > Oops... I just did a blind replace there, thanks for spotting. > But a meta-comment, should stuff like this go directly into the .c files > itself so that the documentation is created automatically? the fact > that this lives so "far away" from the source ensures that it will > always be out of date. I know other subsystems (graphics, v4l2) have > tied the documentation into their code files much better so I think the > build and markup infrastructure is there today to do this. > Well you're right that this documentation is far from its implementation but that applies to most of the stuffs inside kernel, right? Also, I think if we move these into .c file, then it will flood the whole file IMO. We already have the kernel doc for most of the APIs/structures and that should be enough for the .c/.h code in my perspective. If a developer wants to obtain more information other than the API/struct definitions, then they should land in documentation. It should be responsibility of the maintainer to keep the doc up-to date :) Thanks, Mani > thanks, > > greg k-h
[PATCH] docs: driver-model: Update the documentation for device class
The current documentation about the device class is out of date such
that it refers to non-existent APIs and structures. This commit updates
them to the current device class APIs and structures, removes wordings
that no longer valid while trying to keep the original content intact.
Signed-off-by: Manivannan Sadhasivam
---
.../driver-api/driver-model/class.rst | 144 --
1 file changed, 66 insertions(+), 78 deletions(-)
diff --git a/Documentation/driver-api/driver-model/class.rst
b/Documentation/driver-api/driver-model/class.rst
index fff55b80e86a..4e1779a37939 100644
--- a/Documentation/driver-api/driver-model/class.rst
+++ b/Documentation/driver-api/driver-model/class.rst
@@ -5,12 +5,7 @@ Device Classes
Introduction
A device class describes a type of device, like an audio or network
-device. The following device classes have been identified:
-
-
-
-
-Each device class defines a set of semantics and a programming interface
+device. It defines a set of semantics and a programming interface
that devices of that class adhere to. Device drivers are the
implementation of that programming interface for a particular device on
a particular bus.
@@ -18,23 +13,27 @@ a particular bus.
Device classes are agnostic with respect to what bus a device resides
on.
-
Programming Interface
~
The device class structure looks like::
+ struct class {
+const char *name;
+struct module *owner;
- typedef int (*devclass_add)(struct device *);
- typedef void (*devclass_remove)(struct device *);
+const struct attribute_group**class_groups;
+const struct attribute_group**dev_groups;
+struct kobject *dev_kobj;
+...
+ };
See the kerneldoc for the struct class.
A typical device class definition would look like::
- struct device_class input_devclass = {
-.name = "input",
-.add_device= input_add_device,
- .remove_device = input_remove_device,
+ struct class input_class = {
+.name = "input",
+.dev_release= input_dev_release,
};
Each device class structure should be exported in a header file so it
@@ -42,101 +41,84 @@ can be used by drivers, extensions and interfaces.
Device classes are registered and unregistered with the core using::
- int devclass_register(struct device_class * cls);
- void devclass_unregister(struct device_class * cls);
-
+ int class_register(struct class *class);
+ void class_unregister(struct class *class);
Devices
~~~
-As devices are bound to drivers, they are added to the device class
-that the driver belongs to. Before the driver model core, this would
-typically happen during the driver's probe() callback, once the device
-has been initialized. It now happens after the probe() callback
-finishes from the core.
-
-The device is enumerated in the class. Each time a device is added to
-the class, the class's devnum field is incremented and assigned to the
-device. The field is never decremented, so if the device is removed
-from the class and re-added, it will receive a different enumerated
-value.
-
-The class is allowed to create a class-specific structure for the
-device and store it in the device's class_data pointer.
-
-There is no list of devices in the device class. Each driver has a
-list of devices that it supports. The device class has a list of
-drivers of that particular class. To access all of the devices in the
-class, iterate over the device lists of each driver in the class.
+When a device is added, it is also added to the 'klist_devices' inside
+the 'subsys_private' struct of the class. Later, the devices belonging
+to the class are accessed using::
+ class_dev_iter_next()
+ class_find_device()
+ class_find_device_by_name()
-Device Drivers
-~~
-Device drivers are added to device classes when they are registered
-with the core. A driver specifies the class it belongs to by setting
-the struct device_driver::devclass field.
+It is also possible to access the devices of a class in a platform
+dependent way using::
+ class_find_device_by_of_node()
+ class_find_device_by_acpi_dev()
sysfs directory structure
There is a top-level sysfs directory named 'class'.
-Each class gets a directory in the class directory, along with two
-default subdirectories::
+Each class gets a directory in the top-level class directory::
-class/
-`-- input
-|-- devices
-`-- drivers
+ class/
+ |-- input
+ |-- block
+ |-- drm
+ |-- nvme
-
-Drivers registered with the class get a symlink in the drivers/ directory
-that points to the driver's directory (under its bus directory)::
-
- class/
- `-- input
- |-- devices
- `-- drivers
- `-- usb:usb_mo
Re: [PATCH v2 3/6] soc: actions: Add Actions Semi Owl socinfo driver
On Tue, Mar 30, 2021 at 04:48:18PM +0300, Cristian Ciocaltea wrote:
> The driver provides information about the Action Semi Owl family of
> SoCs (S500, S700 and S900) to user space via sysfs: machine, family,
> soc_id, serial_number.
>
> Note the serial number is currently provided only for the S500 SoC
> variant.
>
> Signed-off-by: Cristian Ciocaltea
> ---
> drivers/soc/actions/Kconfig | 8 +
> drivers/soc/actions/Makefile | 1 +
> drivers/soc/actions/owl-socinfo.c | 152 ++
> include/linux/soc/actions/owl-serial-number.h | 20 +++
> 4 files changed, 181 insertions(+)
> create mode 100644 drivers/soc/actions/owl-socinfo.c
> create mode 100644 include/linux/soc/actions/owl-serial-number.h
>
> diff --git a/drivers/soc/actions/Kconfig b/drivers/soc/actions/Kconfig
> index 1aca2058a40c..15faade9282d 100644
> --- a/drivers/soc/actions/Kconfig
> +++ b/drivers/soc/actions/Kconfig
> @@ -14,4 +14,12 @@ config OWL_PM_DOMAINS
> power-gating on Actions Semiconductor S500, S700 and S900 SoCs.
> If unsure, say 'n'.
>
> +config OWL_SOCINFO
> + bool "Actions Semi Owl SoC info driver"
> + default ARCH_ACTIONS
> + select SOC_BUS
> + help
> + Say 'y' here to support the Action Semiconductor Owl socinfo
Actions Semi
> + driver, providing information about the SoC to user space.
> +
> endif
> diff --git a/drivers/soc/actions/Makefile b/drivers/soc/actions/Makefile
> index 4db9e7b050e5..4b2591d3089f 100644
> --- a/drivers/soc/actions/Makefile
> +++ b/drivers/soc/actions/Makefile
> @@ -2,3 +2,4 @@
>
> obj-$(CONFIG_OWL_PM_DOMAINS_HELPER) += owl-sps-helper.o
> obj-$(CONFIG_OWL_PM_DOMAINS) += owl-sps.o
> +obj-$(CONFIG_OWL_SOCINFO) += owl-socinfo.o
> diff --git a/drivers/soc/actions/owl-socinfo.c
> b/drivers/soc/actions/owl-socinfo.c
> new file mode 100644
> index ..f28eafac3792
> --- /dev/null
> +++ b/drivers/soc/actions/owl-socinfo.c
> @@ -0,0 +1,152 @@
[...]
> + * Access SoC's serial number stored by the bootloader in DDR memory.
> + */
> +static int owl_socinfo_read_serial_rmem(struct device *dev)
> +{
> + struct reserved_mem *rmem;
> + struct device_node *np;
> + int ret = 0;
> +
> + np = of_find_compatible_node(NULL, NULL, "actions,owl-soc-serial");
> + if (!np)
> + return -ENXIO;
> +
> + rmem = of_reserved_mem_lookup(np);
If you do this correctly, you could use "pdev->dev.of_node" here instead of
using "of_find_compatible_node()" for getting np.
> + if (!rmem) {
> + dev_err(dev, "failed to acquire reserved memory region\n");
> + ret = -EINVAL;
> + goto out_put;
> + }
> +
[...]
> +static const struct of_device_id owl_socinfo_of_match[] = {
> + { .compatible = "actions,s500-soc", .data = &s500_soc_info, },
> + { .compatible = "actions,s700-soc", .data = &s700_soc_info, },
> + { .compatible = "actions,s900-soc", .data = &s900_soc_info, },
Please don't add S700/S900 for now.
> + { }
> +};
> +
> +static struct platform_driver owl_socinfo_platform_driver = {
> + .probe = owl_socinfo_probe,
> + .driver = {
> + .name = "owl-socinfo",
> + .of_match_table = owl_socinfo_of_match,
> + },
> +};
> +
> +static int __init owl_socinfo_init(void)
> +{
> + return platform_driver_register(&owl_socinfo_platform_driver);
> +}
> +subsys_initcall(owl_socinfo_init);
> diff --git a/include/linux/soc/actions/owl-serial-number.h
> b/include/linux/soc/actions/owl-serial-number.h
> new file mode 100644
> index ..f8595417668f
> --- /dev/null
> +++ b/include/linux/soc/actions/owl-serial-number.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2021 Cristian Ciocaltea
> + */
> +
> +#ifndef __SOC_ACTIONS_OWL_SERIAL_NUMBER_H__
> +#define __SOC_ACTIONS_OWL_SERIAL_NUMBER_H__
> +
> +#if IS_ENABLED(CONFIG_OWL_SOCINFO)
> +u32 owl_get_soc_serial_low(void);
> +u32 owl_get_soc_serial_high(void);
Where are these APIs used?
Thanks,
Mani
> +#else
> +static inline u32 owl_get_soc_serial_low(void)
> +{ return 0; }
> +
> +static inline u32 owl_get_soc_serial_high(void)
> +{ return 0; }
> +#endif /* CONFIG_OWL_SOCINFO */
> +
> +#endif /* __SOC_ACTIONS_OWL_SERIAL_NUMBER_H__ */
> --
> 2.31.1
>
Re: [PATCH v2 1/6] dt-bindings: reserved-memory: Add Owl SoC serial number binding
On Thu, Apr 01, 2021 at 08:40:01PM +0300, Cristian Ciocaltea wrote:
> On Thu, Apr 01, 2021 at 12:07:04PM -0500, Rob Herring wrote:
> > On Tue, Mar 30, 2021 at 04:48:16PM +0300, Cristian Ciocaltea wrote:
> > > Add devicetree binding for the Actions Semi Owl SoC serial number
> > > reserved-memory range.
> > >
> > > Signed-off-by: Cristian Ciocaltea
> > > ---
> > > .../actions,owl-soc-serial.yaml | 53 +++
> > > 1 file changed, 53 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/reserved-memory/actions,owl-soc-serial.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/reserved-memory/actions,owl-soc-serial.yaml
> > >
> > > b/Documentation/devicetree/bindings/reserved-memory/actions,owl-soc-serial.yaml
> > > new file mode 100644
> > > index ..41b71f47ee6c
> > > --- /dev/null
> > > +++
> > > b/Documentation/devicetree/bindings/reserved-memory/actions,owl-soc-serial.yaml
> > > @@ -0,0 +1,53 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:
> > > http://devicetree.org/schemas/reserved-memory/actions,owl-soc-serial.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Actions Semi Owl reserved-memory for SoC serial number
> > > +
> > > +maintainers:
> > > + - Cristian Ciocaltea
> > > +
> > > +description: |
> > > + Provide access to the memory region where the two parts of the Actions
> > > + Semi Owl SoC serial number (low & high) can be read from. This
> > > information
> > > + is provided by the bootloader, hence expose it under /reserved-memory
> > > node.
> > > +
> > > + Please refer to reserved-memory.txt in this directory for common
> > > binding
> > > + part and usage.
> > > +
> > > + This is currently supported only on the S500 SoC variant.
> > > +
> > > +properties:
> > > + compatible:
> > > +oneOf:
> > > + - const: actions,owl-soc-serial
> > > + - items:
> > > + - enum:
> > > + - actions,s500-soc-serial
> > > + - const: actions,owl-soc-serial
> > > +
> > > + reg:
> > > +maxItems: 1
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > +
> > > +additionalProperties: true
> > > +
> > > +examples:
> > > + - |
> > > +reserved-memory {
> > > +#address-cells = <1>;
> > > +#size-cells = <1>;
> > > +ranges;
> > > +
> > > +soc_serial: soc-serial@800 {
> > > +compatible = "actions,s500-soc-serial",
> > > "actions,owl-soc-serial";
> > > +reg = <0x800 0x8>;
> >
> > You end up wasting a whole page of memory for 8 bytes. It may be better
> > to copy this to a DT property ('serial-number' is already a defined root
> > property).
>
> Actually there is more information provided by the vendor bootloader
Then you should call it as socinfo or something not soc_serial.
Thanks,
Mani
> in this memory page, so we might use it once we are able to decode it.
> For the moment I could only identify the serial number.
>
> Thanks,
> Cristi
>
> > Rob
Re: [PATCH v2 2/6] dt-bindings: soc: actions: Add Actions Semi Owl socinfo binding
On Tue, Mar 30, 2021 at 04:48:17PM +0300, Cristian Ciocaltea wrote:
> Add devicetree binding for the Actions Semi Owl socinfo driver.
>
Devicetree binding shouldn't be added for a driver instead for an IP or hw.
> Signed-off-by: Cristian Ciocaltea
> ---
> .../bindings/soc/actions/owl-socinfo.yaml | 57 +++
> 1 file changed, 57 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> new file mode 100644
> index ..01e4a8b4f5ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/actions/owl-socinfo.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Actions Semi Owl SoC info module
> +
> +maintainers:
> + - Cristian Ciocaltea
> +
> +description: |
> + Actions Semi Owl SoC info module provides access to various information
> + about the S500, S700 and S900 SoC variants, such as serial number or id.
> +
S700/S900 are not yet confirmed, so please avoid them.
> +select:
> + properties:
> +compatible:
> + contains:
> +enum:
> + - actions,s500-soc
> + - actions,s700-soc
> + - actions,s900-soc
> + required:
> +- compatible
> +
> +properties:
> + compatible:
> +items:
> + - enum:
> + - actions,s500-soc
> + - actions,s700-soc
> + - actions,s900-soc
> + - const: simple-bus
> +
> +required:
> + - compatible
> +
> +additionalProperties: true
> +
> +examples:
> + - |
> +/ {
> +compatible = "roseapplepi,roseapplepi", "actions,s500";
> +model = "Roseapple Pi";
> +#address-cells = <1>;
> +#size-cells = <1>;
> +
> +soc {
> +compatible = "actions,s500-soc", "simple-bus";
No. This shouldn't fall under /soc. I think you should added a separate
compatible for the reserved memory itself and add a corresponding socinfo
driver under drivers/soc.
This way it is obvious that the information is contained in a memory region and
a driver is used for parsing that.
Thanks,
Mani
> +#address-cells = <1>;
> +#size-cells = <1>;
> +ranges;
> +};
> +};
> +
> +...
> --
> 2.31.1
>
[PATCH v11 3/4] mtd: rawnand: Add support for secure regions in NAND memory
On a typical end product, a vendor may choose to secure some regions in
the NAND memory which are supposed to stay intact between FW upgrades.
The access to those regions will be blocked by a secure element like
Trustzone. So the normal world software like Linux kernel should not
touch these regions (including reading).
The regions are declared using a NAND chip DT property,
"secure-regions". So let's make use of this property in the raw NAND
core and skip access to the secure regions present in a system.
Signed-off-by: Manivannan Sadhasivam
---
drivers/mtd/nand/raw/nand_base.c | 100 ++-
include/linux/mtd/rawnand.h | 14 +
2 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c33fa1b1847f..132ce73df76b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -278,11 +278,48 @@ static int nand_block_bad(struct nand_chip *chip, loff_t
ofs)
return 0;
}
+/**
+ * nand_region_is_secured() - Check if the region is secured
+ * @chip: NAND chip object
+ * @offset: Offset of the region to check
+ * @size: Size of the region to check
+ *
+ * Checks if the region is secured by comparing the offset and size with the
+ * list of secure regions obtained from DT. Returns true if the region is
+ * secured else false.
+ */
+static bool nand_region_is_secured(struct nand_chip *chip, loff_t offset, u64
size)
+{
+ int i;
+
+ /* Skip touching the secure regions if present */
+ for (i = 0; i < chip->nr_secure_regions; i++) {
+ const struct nand_secure_region *region =
&chip->secure_regions[i];
+
+ if (offset + size <= region->offset ||
+ offset >= region->offset + region->size)
+ continue;
+
+ pr_debug("%s: Region 0x%llx - 0x%llx is secured!",
+__func__, offset, offset + size);
+
+ return true;
+ }
+
+ return false;
+}
+
static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+
if (chip->options & NAND_NO_BBM_QUIRK)
return 0;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, ofs, mtd->erasesize))
+ return -EIO;
+
if (chip->legacy.block_bad)
return chip->legacy.block_bad(chip, ofs);
@@ -397,6 +434,10 @@ static int nand_do_write_oob(struct nand_chip *chip,
loff_t to,
return -EINVAL;
}
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, to, ops->ooblen))
+ return -EIO;
+
chipnr = (int)(to >> chip->chip_shift);
/*
@@ -3127,6 +3168,10 @@ static int nand_do_read_ops(struct nand_chip *chip,
loff_t from,
int retry_mode = 0;
bool ecc_fail = false;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, from, readlen))
+ return -EIO;
+
chipnr = (int)(from >> chip->chip_shift);
nand_select_target(chip, chipnr);
@@ -3458,6 +3503,10 @@ static int nand_do_read_oob(struct nand_chip *chip,
loff_t from,
pr_debug("%s: from = 0x%08Lx, len = %i\n",
__func__, (unsigned long long)from, readlen);
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, from, readlen))
+ return -EIO;
+
stats = mtd->ecc_stats;
len = mtd_oobavail(mtd, ops);
@@ -3979,6 +4028,10 @@ static int nand_do_write_ops(struct nand_chip *chip,
loff_t to,
return -EINVAL;
}
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, to, writelen))
+ return -EIO;
+
column = to & (mtd->writesize - 1);
chipnr = (int)(to >> chip->chip_shift);
@@ -4180,6 +4233,10 @@ int nand_erase_nand(struct nand_chip *chip, struct
erase_info *instr,
if (check_offs_len(chip, instr->addr, instr->len))
return -EINVAL;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, instr->addr, instr->len))
+ return -EIO;
+
/* Grab the lock and see if the device is available */
ret = nand_get_device(chip);
if (ret)
@@ -4995,6 +5052,31 @@ static bool of_get_nand_on_flash_bbt(struct device_node
*np)
return of_property_read_bool(np, "nand-on-flash-bbt");
}
+static int of_get_nand_secure_regions(struct nand_chip *chip)
+{
+ struct device_node *dn = nand_get_flash_node(chip);
+ int nr_elem, i, j;
+
+ nr_elem = of_property_count_elems_of_size(dn, "secure-regions",
sizeof(u64));
+ if (!nr_elem)
+ r
[PATCH v11 4/4] mtd: rawnand: qcom: Add missing nand_cleanup() in error path
Add missing nand_cleanup() in the alloc_bam_transaction() error path
to cleanup the resources properly.
Signed-off-by: Manivannan Sadhasivam
---
drivers/mtd/nand/raw/qcom_nandc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
b/drivers/mtd/nand/raw/qcom_nandc.c
index 87c23bb320bf..fe74cf3aece5 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2882,6 +2882,7 @@ static int qcom_nand_host_init_and_register(struct
qcom_nand_controller *nandc,
if (!nandc->bam_txn) {
dev_err(nandc->dev,
"failed to allocate bam transaction\n");
+ nand_cleanup(chip);
return -ENOMEM;
}
}
--
2.25.1
[PATCH v11 2/4] dt-bindings: mtd: Add a property to declare secure regions in NAND chips
On a typical end product, a vendor may choose to secure some regions in the NAND memory which are supposed to stay intact between FW upgrades. The access to those regions will be blocked by a secure element like Trustzone. So the normal world software like Linux kernel should not touch these regions (including reading). So let's add a property for declaring such secure regions so that the drivers can skip touching them. Reviewed-by: Rob Herring Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml index d0e422f4b3e0..678b39952502 100644 --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml @@ -143,6 +143,13 @@ patternProperties: Ready/Busy pins. Active state refers to the NAND ready state and should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted. + secure-regions: +$ref: /schemas/types.yaml#/definitions/uint64-matrix +description: + Regions in the NAND chip which are protected using a secure element + like Trustzone. This property contains the start address and size of + the secure regions present. + required: - reg -- 2.25.1
[PATCH v11 1/4] dt-bindings: mtd: Convert Qcom NANDc binding to YAML
Convert Qcom NANDc devicetree binding to YAML.
Signed-off-by: Manivannan Sadhasivam
Reviewed-by: Rob Herring
---
.../devicetree/bindings/mtd/qcom,nandc.yaml | 196 ++
.../devicetree/bindings/mtd/qcom_nandc.txt| 142 -
2 files changed, 196 insertions(+), 142 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
diff --git a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
new file mode 100644
index ..84ad7ff30121
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/qcom,nandc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm NAND controller
+
+maintainers:
+ - Manivannan Sadhasivam
+
+properties:
+ compatible:
+enum:
+ - qcom,ipq806x-nand
+ - qcom,ipq4019-nand
+ - qcom,ipq6018-nand
+ - qcom,ipq8074-nand
+ - qcom,sdx55-nand
+
+ reg:
+maxItems: 1
+
+ clocks:
+items:
+ - description: Core Clock
+ - description: Always ON Clock
+
+ clock-names:
+items:
+ - const: core
+ - const: aon
+
+ "#address-cells": true
+ "#size-cells": true
+
+patternProperties:
+ "^nand@[a-f0-9]$":
+type: object
+properties:
+ nand-bus-width:
+const: 8
+
+ nand-ecc-strength:
+enum: [1, 4, 8]
+
+ nand-ecc-step-size:
+enum:
+ - 512
+
+allOf:
+ - $ref: "nand-controller.yaml#"
+
+ - if:
+ properties:
+compatible:
+ contains:
+const: qcom,ipq806x-nand
+then:
+ properties:
+dmas:
+ items:
+- description: rxtx DMA channel
+
+dma-names:
+ items:
+- const: rxtx
+
+qcom,cmd-crci:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+Must contain the ADM command type CRCI block instance number
+specified for the NAND controller on the given platform
+
+qcom,data-crci:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+Must contain the ADM data type CRCI block instance number
+specified for the NAND controller on the given platform
+
+ - if:
+ properties:
+compatible:
+ contains:
+enum:
+ - qcom,ipq4019-nand
+ - qcom,ipq6018-nand
+ - qcom,ipq8074-nand
+ - qcom,sdx55-nand
+
+then:
+ properties:
+dmas:
+ items:
+- description: tx DMA channel
+- description: rx DMA channel
+- description: cmd DMA channel
+
+dma-names:
+ items:
+- const: tx
+- const: rx
+- const: cmd
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+nand-controller@1ac0 {
+ compatible = "qcom,ipq806x-nand";
+ reg = <0x1ac0 0x800>;
+
+ clocks = <&gcc EBI2_CLK>,
+ <&gcc EBI2_AON_CLK>;
+ clock-names = "core", "aon";
+
+ dmas = <&adm_dma 3>;
+ dma-names = "rxtx";
+ qcom,cmd-crci = <15>;
+ qcom,data-crci = <3>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+reg = <0>;
+
+nand-ecc-strength = <4>;
+nand-bus-width = <8>;
+
+partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+label = "boot-nand";
+reg = <0 0x58a>;
+ };
+
+ partition@58a {
+label = "fs-nand";
+reg = <0x58a 0x400>;
+ };
+};
+ };
+};
+
+#include
+nand-controller@79b {
+ compatible = "qcom,ipq4019-nand";
+ reg = <0x79b 0x1000>;
+
+ clocks = <&gcc GCC_QPIC_CLK>,
+ <&gcc GCC_QPIC_AHB_CLK>;
+ clock-names = "core", "aon";
+
+ dmas = <&qpicbam 0>,
+ <&qpicbam 1>,
+ <&qpicbam 2>;
+ dma-names = "tx", "rx", "cmd";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+reg = <0>;
+nand-ecc-strength = <4>;
+nand-bus-width = <8>;
+
+partitions {
+ compatible = "fixed-partitions";
+
[PATCH v11 0/4] Add support for secure regions in NAND
On a typical end product, a vendor may choose to secure some regions in the NAND memory which are supposed to stay intact between FW upgrades. The access to those regions will be blocked by a secure element like Trustzone. So the normal world software like Linux kernel should not touch these regions (including reading). So this series adds a property for declaring such secure regions in DT so that the driver can skip touching them. While at it, the Qcom NANDc DT binding is also converted to YAML format. Thanks, Mani Changes in v11: * Removed unneeded check in nand_block_isreserved() * Used mtd->erasesize as the size in nand_isbad_bbm() Changes in v10: * Added Rob's review tag for binding Changes in v9: Based on review comments from Miquel: * Fixed the secure-regions check * Renamed the function to nand_region_is_secured() and used bool return * Moved the parsing function to nand_scan() * Added a patch to fix nand_cleanup in qcom driver Changes in v8: * Reworked the secure region check logic based on input from Boris * Removed the check where unnecessary in rawnand core. Changes in v7: * Made "size" u64 and fixed a warning reported by Kernel test bot Changes in v6: * Made use of "size" of the regions for comparision * Used "secure" instead of "sec" * Fixed the sizeof parameter in of_get_nand_secure_regions() Changes in v5: * Switched to "uint64-matrix" as suggested by Rob * Moved the whole logic from qcom driver to nand core as suggested by Boris Changes in v4: * Used "uint32-matrix" instead of "uint32-array" as per Rob's review. * Collected Rob's review tag for binding conversion patch Changes in v3: * Removed the nand prefix from DT property and moved the property parsing logic before nand_scan() in driver. Changes in v2: * Moved the secure-regions property to generic NAND binding as a NAND chip property and renamed it as "nand-secure-regions". Manivannan Sadhasivam (4): dt-bindings: mtd: Convert Qcom NANDc binding to YAML dt-bindings: mtd: Add a property to declare secure regions in NAND chips mtd: rawnand: Add support for secure regions in NAND memory mtd: rawnand: qcom: Add missing nand_cleanup() in error path .../bindings/mtd/nand-controller.yaml | 7 + .../devicetree/bindings/mtd/qcom,nandc.yaml | 196 ++ .../devicetree/bindings/mtd/qcom_nandc.txt| 142 - drivers/mtd/nand/raw/nand_base.c | 100 - drivers/mtd/nand/raw/qcom_nandc.c | 1 + include/linux/mtd/rawnand.h | 14 ++ 6 files changed, 317 insertions(+), 143 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt -- 2.25.1
Re: [PATCH v10 3/4] mtd: rawnand: Add support for secure regions in NAND memory
On Fri, Apr 02, 2021 at 10:51:54AM +0200, Boris Brezillon wrote: > On Thu, 1 Apr 2021 21:46:22 +0530 > Manivannan Sadhasivam wrote: > > > On Thu, Apr 01, 2021 at 05:54:21PM +0200, Boris Brezillon wrote: > > > On Thu, 1 Apr 2021 20:49:54 +0530 > > > Manivannan Sadhasivam wrote: > > > > > > > @@ -565,6 +608,11 @@ static int nand_block_isreserved(struct mtd_info > > > > *mtd, loff_t ofs) > > > > > > > > if (!chip->bbt) > > > > return 0; > > > > + > > > > + /* Check if the region is secured */ > > > > + if (nand_region_is_secured(chip, ofs, 0)) > > > > + return -EIO; > > > > > > That would is still wrong, you should never pass a 0 size to > > > nand_region_is_secured(). > > > > > > > Size doesn't matter here, that's why I passed 0. Maybe 1 would be > > appropriate? > > You're checking if a block is reserved, so I think passing the > eraseblock size would make more sense, but I actually don't understand > why you need to check if the region is secure here (looks like > nand_block_isreserved() does not access the flash). > Ah yes indeed, brain fade... Thanks, Mani
[PATCH v10 4/4] mtd: rawnand: qcom: Add missing nand_cleanup() in error path
Add missing nand_cleanup() in the alloc_bam_transaction() error path
to cleanup the resources properly.
Signed-off-by: Manivannan Sadhasivam
---
drivers/mtd/nand/raw/qcom_nandc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
b/drivers/mtd/nand/raw/qcom_nandc.c
index 87c23bb320bf..fe74cf3aece5 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2882,6 +2882,7 @@ static int qcom_nand_host_init_and_register(struct
qcom_nand_controller *nandc,
if (!nandc->bam_txn) {
dev_err(nandc->dev,
"failed to allocate bam transaction\n");
+ nand_cleanup(chip);
return -ENOMEM;
}
}
--
2.25.1
[PATCH v10 2/4] dt-bindings: mtd: Add a property to declare secure regions in NAND chips
On a typical end product, a vendor may choose to secure some regions in the NAND memory which are supposed to stay intact between FW upgrades. The access to those regions will be blocked by a secure element like Trustzone. So the normal world software like Linux kernel should not touch these regions (including reading). So let's add a property for declaring such secure regions so that the drivers can skip touching them. Reviewed-by: Rob Herring Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml index d0e422f4b3e0..678b39952502 100644 --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml @@ -143,6 +143,13 @@ patternProperties: Ready/Busy pins. Active state refers to the NAND ready state and should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted. + secure-regions: +$ref: /schemas/types.yaml#/definitions/uint64-matrix +description: + Regions in the NAND chip which are protected using a secure element + like Trustzone. This property contains the start address and size of + the secure regions present. + required: - reg -- 2.25.1
Re: [PATCH v10 3/4] mtd: rawnand: Add support for secure regions in NAND memory
On Thu, Apr 01, 2021 at 05:54:21PM +0200, Boris Brezillon wrote: > On Thu, 1 Apr 2021 20:49:54 +0530 > Manivannan Sadhasivam wrote: > > > @@ -565,6 +608,11 @@ static int nand_block_isreserved(struct mtd_info *mtd, > > loff_t ofs) > > > > if (!chip->bbt) > > return 0; > > + > > + /* Check if the region is secured */ > > + if (nand_region_is_secured(chip, ofs, 0)) > > + return -EIO; > > That would is still wrong, you should never pass a 0 size to > nand_region_is_secured(). > Size doesn't matter here, that's why I passed 0. Maybe 1 would be appropriate? Thanks, Mani
[PATCH v9 0/4] Add support for secure regions in NAND
On a typical end product, a vendor may choose to secure some regions in the NAND memory which are supposed to stay intact between FW upgrades. The access to those regions will be blocked by a secure element like Trustzone. So the normal world software like Linux kernel should not touch these regions (including reading). So this series adds a property for declaring such secure regions in DT so that the driver can skip touching them. While at it, the Qcom NANDc DT binding is also converted to YAML format. Thanks, Mani Changes in v9: Based on review comments from Miquel: * Fixed the secure-regions check * Renamed the function to nand_region_is_secured() and used bool return * Moved the parsing function to nand_scan() * Added a patch to fix nand_cleanup in qcom driver Changes in v8: * Reworked the secure region check logic based on input from Boris * Removed the check where unnecessary in rawnand core. Changes in v7: * Made "size" u64 and fixed a warning reported by Kernel test bot Changes in v6: * Made use of "size" of the regions for comparision * Used "secure" instead of "sec" * Fixed the sizeof parameter in of_get_nand_secure_regions() Changes in v5: * Switched to "uint64-matrix" as suggested by Rob * Moved the whole logic from qcom driver to nand core as suggested by Boris Changes in v4: * Used "uint32-matrix" instead of "uint32-array" as per Rob's review. * Collected Rob's review tag for binding conversion patch Changes in v3: * Removed the nand prefix from DT property and moved the property parsing logic before nand_scan() in driver. Changes in v2: * Moved the secure-regions property to generic NAND binding as a NAND chip property and renamed it as "nand-secure-regions". Manivannan Sadhasivam (4): dt-bindings: mtd: Convert Qcom NANDc binding to YAML dt-bindings: mtd: Add a property to declare secure regions in NAND chips mtd: rawnand: Add support for secure regions in NAND memory mtd: rawnand: qcom: Add missing nand_cleanup() in error path .../bindings/mtd/nand-controller.yaml | 7 + .../devicetree/bindings/mtd/qcom,nandc.yaml | 196 ++ .../devicetree/bindings/mtd/qcom_nandc.txt| 142 - drivers/mtd/nand/raw/nand_base.c | 107 +- drivers/mtd/nand/raw/qcom_nandc.c | 1 + include/linux/mtd/rawnand.h | 14 ++ 6 files changed, 324 insertions(+), 143 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt -- 2.25.1
[PATCH v10 0/4] Add support for secure regions in NAND
On a typical end product, a vendor may choose to secure some regions in the NAND memory which are supposed to stay intact between FW upgrades. The access to those regions will be blocked by a secure element like Trustzone. So the normal world software like Linux kernel should not touch these regions (including reading). So this series adds a property for declaring such secure regions in DT so that the driver can skip touching them. While at it, the Qcom NANDc DT binding is also converted to YAML format. Thanks, Mani Changes in v10: * Added Rob's review tag for binding Changes in v9: Based on review comments from Miquel: * Fixed the secure-regions check * Renamed the function to nand_region_is_secured() and used bool return * Moved the parsing function to nand_scan() * Added a patch to fix nand_cleanup in qcom driver Changes in v8: * Reworked the secure region check logic based on input from Boris * Removed the check where unnecessary in rawnand core. Changes in v7: * Made "size" u64 and fixed a warning reported by Kernel test bot Changes in v6: * Made use of "size" of the regions for comparision * Used "secure" instead of "sec" * Fixed the sizeof parameter in of_get_nand_secure_regions() Changes in v5: * Switched to "uint64-matrix" as suggested by Rob * Moved the whole logic from qcom driver to nand core as suggested by Boris Changes in v4: * Used "uint32-matrix" instead of "uint32-array" as per Rob's review. * Collected Rob's review tag for binding conversion patch Changes in v3: * Removed the nand prefix from DT property and moved the property parsing logic before nand_scan() in driver. Changes in v2: * Moved the secure-regions property to generic NAND binding as a NAND chip property and renamed it as "nand-secure-regions". Manivannan Sadhasivam (4): dt-bindings: mtd: Convert Qcom NANDc binding to YAML dt-bindings: mtd: Add a property to declare secure regions in NAND chips mtd: rawnand: Add support for secure regions in NAND memory mtd: rawnand: qcom: Add missing nand_cleanup() in error path .../bindings/mtd/nand-controller.yaml | 7 + .../devicetree/bindings/mtd/qcom,nandc.yaml | 196 ++ .../devicetree/bindings/mtd/qcom_nandc.txt| 142 - drivers/mtd/nand/raw/nand_base.c | 107 +- drivers/mtd/nand/raw/qcom_nandc.c | 1 + include/linux/mtd/rawnand.h | 14 ++ 6 files changed, 324 insertions(+), 143 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt -- 2.25.1
[PATCH v10 1/4] dt-bindings: mtd: Convert Qcom NANDc binding to YAML
Convert Qcom NANDc devicetree binding to YAML.
Signed-off-by: Manivannan Sadhasivam
Reviewed-by: Rob Herring
---
.../devicetree/bindings/mtd/qcom,nandc.yaml | 196 ++
.../devicetree/bindings/mtd/qcom_nandc.txt| 142 -
2 files changed, 196 insertions(+), 142 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
diff --git a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
new file mode 100644
index ..84ad7ff30121
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/qcom,nandc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm NAND controller
+
+maintainers:
+ - Manivannan Sadhasivam
+
+properties:
+ compatible:
+enum:
+ - qcom,ipq806x-nand
+ - qcom,ipq4019-nand
+ - qcom,ipq6018-nand
+ - qcom,ipq8074-nand
+ - qcom,sdx55-nand
+
+ reg:
+maxItems: 1
+
+ clocks:
+items:
+ - description: Core Clock
+ - description: Always ON Clock
+
+ clock-names:
+items:
+ - const: core
+ - const: aon
+
+ "#address-cells": true
+ "#size-cells": true
+
+patternProperties:
+ "^nand@[a-f0-9]$":
+type: object
+properties:
+ nand-bus-width:
+const: 8
+
+ nand-ecc-strength:
+enum: [1, 4, 8]
+
+ nand-ecc-step-size:
+enum:
+ - 512
+
+allOf:
+ - $ref: "nand-controller.yaml#"
+
+ - if:
+ properties:
+compatible:
+ contains:
+const: qcom,ipq806x-nand
+then:
+ properties:
+dmas:
+ items:
+- description: rxtx DMA channel
+
+dma-names:
+ items:
+- const: rxtx
+
+qcom,cmd-crci:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+Must contain the ADM command type CRCI block instance number
+specified for the NAND controller on the given platform
+
+qcom,data-crci:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+Must contain the ADM data type CRCI block instance number
+specified for the NAND controller on the given platform
+
+ - if:
+ properties:
+compatible:
+ contains:
+enum:
+ - qcom,ipq4019-nand
+ - qcom,ipq6018-nand
+ - qcom,ipq8074-nand
+ - qcom,sdx55-nand
+
+then:
+ properties:
+dmas:
+ items:
+- description: tx DMA channel
+- description: rx DMA channel
+- description: cmd DMA channel
+
+dma-names:
+ items:
+- const: tx
+- const: rx
+- const: cmd
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+nand-controller@1ac0 {
+ compatible = "qcom,ipq806x-nand";
+ reg = <0x1ac0 0x800>;
+
+ clocks = <&gcc EBI2_CLK>,
+ <&gcc EBI2_AON_CLK>;
+ clock-names = "core", "aon";
+
+ dmas = <&adm_dma 3>;
+ dma-names = "rxtx";
+ qcom,cmd-crci = <15>;
+ qcom,data-crci = <3>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+reg = <0>;
+
+nand-ecc-strength = <4>;
+nand-bus-width = <8>;
+
+partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+label = "boot-nand";
+reg = <0 0x58a>;
+ };
+
+ partition@58a {
+label = "fs-nand";
+reg = <0x58a 0x400>;
+ };
+};
+ };
+};
+
+#include
+nand-controller@79b {
+ compatible = "qcom,ipq4019-nand";
+ reg = <0x79b 0x1000>;
+
+ clocks = <&gcc GCC_QPIC_CLK>,
+ <&gcc GCC_QPIC_AHB_CLK>;
+ clock-names = "core", "aon";
+
+ dmas = <&qpicbam 0>,
+ <&qpicbam 1>,
+ <&qpicbam 2>;
+ dma-names = "tx", "rx", "cmd";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+reg = <0>;
+nand-ecc-strength = <4>;
+nand-bus-width = <8>;
+
+partitions {
+ compatible = "fixed-partitions";
+
[PATCH v9 2/4] dt-bindings: mtd: Add a property to declare secure regions in NAND chips
On a typical end product, a vendor may choose to secure some regions in the NAND memory which are supposed to stay intact between FW upgrades. The access to those regions will be blocked by a secure element like Trustzone. So the normal world software like Linux kernel should not touch these regions (including reading). So let's add a property for declaring such secure regions so that the drivers can skip touching them. Signed-off-by: Manivannan Sadhasivam --- Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml index d0e422f4b3e0..678b39952502 100644 --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml @@ -143,6 +143,13 @@ patternProperties: Ready/Busy pins. Active state refers to the NAND ready state and should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted. + secure-regions: +$ref: /schemas/types.yaml#/definitions/uint64-matrix +description: + Regions in the NAND chip which are protected using a secure element + like Trustzone. This property contains the start address and size of + the secure regions present. + required: - reg -- 2.25.1
[PATCH v9 4/4] mtd: rawnand: qcom: Add missing nand_cleanup() in error path
Add missing nand_cleanup() in the alloc_bam_transaction() error path
to cleanup the resources properly.
Signed-off-by: Manivannan Sadhasivam
---
drivers/mtd/nand/raw/qcom_nandc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
b/drivers/mtd/nand/raw/qcom_nandc.c
index 87c23bb320bf..fe74cf3aece5 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2882,6 +2882,7 @@ static int qcom_nand_host_init_and_register(struct
qcom_nand_controller *nandc,
if (!nandc->bam_txn) {
dev_err(nandc->dev,
"failed to allocate bam transaction\n");
+ nand_cleanup(chip);
return -ENOMEM;
}
}
--
2.25.1
[PATCH v10 3/4] mtd: rawnand: Add support for secure regions in NAND memory
On a typical end product, a vendor may choose to secure some regions in
the NAND memory which are supposed to stay intact between FW upgrades.
The access to those regions will be blocked by a secure element like
Trustzone. So the normal world software like Linux kernel should not
touch these regions (including reading).
The regions are declared using a NAND chip DT property,
"secure-regions". So let's make use of this property in the raw NAND
core and skip access to the secure regions present in a system.
Signed-off-by: Manivannan Sadhasivam
---
drivers/mtd/nand/raw/nand_base.c | 107 ++-
include/linux/mtd/rawnand.h | 14
2 files changed, 120 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c33fa1b1847f..c216d3eca915 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -278,11 +278,50 @@ static int nand_block_bad(struct nand_chip *chip, loff_t
ofs)
return 0;
}
+/**
+ * nand_region_is_secured() - Check if the region is secured
+ * @chip: NAND chip object
+ * @offset: Offset of the region to check
+ * @size: Size of the region to check
+ *
+ * Checks if the region is secured by comparing the offset and size with the
+ * list of secure regions obtained from DT. Returns true if the region is
+ * secured else false.
+ */
+static bool nand_region_is_secured(struct nand_chip *chip, loff_t offset, u64
size)
+{
+ int i;
+
+ /* Skip touching the secure regions if present */
+ for (i = 0; i < chip->nr_secure_regions; i++) {
+ const struct nand_secure_region *region =
&chip->secure_regions[i];
+
+ if (offset + size <= region->offset ||
+ offset >= region->offset + region->size)
+ continue;
+
+ pr_debug("%s: Region 0x%llx - 0x%llx is secured!",
+__func__, offset, offset + size);
+
+ return true;
+ }
+
+ return false;
+}
+
static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ int last_page = ((mtd->erasesize - mtd->writesize) >>
+chip->page_shift) & chip->pagemask;
+
if (chip->options & NAND_NO_BBM_QUIRK)
return 0;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, ofs, last_page))
+ return -EIO;
+
if (chip->legacy.block_bad)
return chip->legacy.block_bad(chip, ofs);
@@ -397,6 +436,10 @@ static int nand_do_write_oob(struct nand_chip *chip,
loff_t to,
return -EINVAL;
}
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, to, ops->ooblen))
+ return -EIO;
+
chipnr = (int)(to >> chip->chip_shift);
/*
@@ -565,6 +608,11 @@ static int nand_block_isreserved(struct mtd_info *mtd,
loff_t ofs)
if (!chip->bbt)
return 0;
+
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, ofs, 0))
+ return -EIO;
+
/* Return info from the table */
return nand_isreserved_bbt(chip, ofs);
}
@@ -3127,6 +3175,10 @@ static int nand_do_read_ops(struct nand_chip *chip,
loff_t from,
int retry_mode = 0;
bool ecc_fail = false;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, from, readlen))
+ return -EIO;
+
chipnr = (int)(from >> chip->chip_shift);
nand_select_target(chip, chipnr);
@@ -3458,6 +3510,10 @@ static int nand_do_read_oob(struct nand_chip *chip,
loff_t from,
pr_debug("%s: from = 0x%08Lx, len = %i\n",
__func__, (unsigned long long)from, readlen);
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, from, readlen))
+ return -EIO;
+
stats = mtd->ecc_stats;
len = mtd_oobavail(mtd, ops);
@@ -3979,6 +4035,10 @@ static int nand_do_write_ops(struct nand_chip *chip,
loff_t to,
return -EINVAL;
}
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, to, writelen))
+ return -EIO;
+
column = to & (mtd->writesize - 1);
chipnr = (int)(to >> chip->chip_shift);
@@ -4180,6 +4240,10 @@ int nand_erase_nand(struct nand_chip *chip, struct
erase_info *instr,
if (check_offs_len(chip, instr->addr, instr->len))
return -EINVAL;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, instr->addr, instr->len))
+ return -EIO;
+
/* Grab the lock and see if the device is available */
[PATCH v9 1/4] dt-bindings: mtd: Convert Qcom NANDc binding to YAML
Convert Qcom NANDc devicetree binding to YAML.
Signed-off-by: Manivannan Sadhasivam
Reviewed-by: Rob Herring
---
.../devicetree/bindings/mtd/qcom,nandc.yaml | 196 ++
.../devicetree/bindings/mtd/qcom_nandc.txt| 142 -
2 files changed, 196 insertions(+), 142 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
diff --git a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
new file mode 100644
index ..84ad7ff30121
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/qcom,nandc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm NAND controller
+
+maintainers:
+ - Manivannan Sadhasivam
+
+properties:
+ compatible:
+enum:
+ - qcom,ipq806x-nand
+ - qcom,ipq4019-nand
+ - qcom,ipq6018-nand
+ - qcom,ipq8074-nand
+ - qcom,sdx55-nand
+
+ reg:
+maxItems: 1
+
+ clocks:
+items:
+ - description: Core Clock
+ - description: Always ON Clock
+
+ clock-names:
+items:
+ - const: core
+ - const: aon
+
+ "#address-cells": true
+ "#size-cells": true
+
+patternProperties:
+ "^nand@[a-f0-9]$":
+type: object
+properties:
+ nand-bus-width:
+const: 8
+
+ nand-ecc-strength:
+enum: [1, 4, 8]
+
+ nand-ecc-step-size:
+enum:
+ - 512
+
+allOf:
+ - $ref: "nand-controller.yaml#"
+
+ - if:
+ properties:
+compatible:
+ contains:
+const: qcom,ipq806x-nand
+then:
+ properties:
+dmas:
+ items:
+- description: rxtx DMA channel
+
+dma-names:
+ items:
+- const: rxtx
+
+qcom,cmd-crci:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+Must contain the ADM command type CRCI block instance number
+specified for the NAND controller on the given platform
+
+qcom,data-crci:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+Must contain the ADM data type CRCI block instance number
+specified for the NAND controller on the given platform
+
+ - if:
+ properties:
+compatible:
+ contains:
+enum:
+ - qcom,ipq4019-nand
+ - qcom,ipq6018-nand
+ - qcom,ipq8074-nand
+ - qcom,sdx55-nand
+
+then:
+ properties:
+dmas:
+ items:
+- description: tx DMA channel
+- description: rx DMA channel
+- description: cmd DMA channel
+
+dma-names:
+ items:
+- const: tx
+- const: rx
+- const: cmd
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+nand-controller@1ac0 {
+ compatible = "qcom,ipq806x-nand";
+ reg = <0x1ac0 0x800>;
+
+ clocks = <&gcc EBI2_CLK>,
+ <&gcc EBI2_AON_CLK>;
+ clock-names = "core", "aon";
+
+ dmas = <&adm_dma 3>;
+ dma-names = "rxtx";
+ qcom,cmd-crci = <15>;
+ qcom,data-crci = <3>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+reg = <0>;
+
+nand-ecc-strength = <4>;
+nand-bus-width = <8>;
+
+partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+label = "boot-nand";
+reg = <0 0x58a>;
+ };
+
+ partition@58a {
+label = "fs-nand";
+reg = <0x58a 0x400>;
+ };
+};
+ };
+};
+
+#include
+nand-controller@79b {
+ compatible = "qcom,ipq4019-nand";
+ reg = <0x79b 0x1000>;
+
+ clocks = <&gcc GCC_QPIC_CLK>,
+ <&gcc GCC_QPIC_AHB_CLK>;
+ clock-names = "core", "aon";
+
+ dmas = <&qpicbam 0>,
+ <&qpicbam 1>,
+ <&qpicbam 2>;
+ dma-names = "tx", "rx", "cmd";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+reg = <0>;
+nand-ecc-strength = <4>;
+nand-bus-width = <8>;
+
+partitions {
+ compatible = "fixed-partitions";
+
[PATCH v9 3/4] mtd: rawnand: Add support for secure regions in NAND memory
On a typical end product, a vendor may choose to secure some regions in
the NAND memory which are supposed to stay intact between FW upgrades.
The access to those regions will be blocked by a secure element like
Trustzone. So the normal world software like Linux kernel should not
touch these regions (including reading).
The regions are declared using a NAND chip DT property,
"secure-regions". So let's make use of this property in the raw NAND
core and skip access to the secure regions present in a system.
Signed-off-by: Manivannan Sadhasivam
---
drivers/mtd/nand/raw/nand_base.c | 107 ++-
include/linux/mtd/rawnand.h | 14
2 files changed, 120 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c33fa1b1847f..c216d3eca915 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -278,11 +278,50 @@ static int nand_block_bad(struct nand_chip *chip, loff_t
ofs)
return 0;
}
+/**
+ * nand_region_is_secured() - Check if the region is secured
+ * @chip: NAND chip object
+ * @offset: Offset of the region to check
+ * @size: Size of the region to check
+ *
+ * Checks if the region is secured by comparing the offset and size with the
+ * list of secure regions obtained from DT. Returns true if the region is
+ * secured else false.
+ */
+static bool nand_region_is_secured(struct nand_chip *chip, loff_t offset, u64
size)
+{
+ int i;
+
+ /* Skip touching the secure regions if present */
+ for (i = 0; i < chip->nr_secure_regions; i++) {
+ const struct nand_secure_region *region =
&chip->secure_regions[i];
+
+ if (offset + size <= region->offset ||
+ offset >= region->offset + region->size)
+ continue;
+
+ pr_debug("%s: Region 0x%llx - 0x%llx is secured!",
+__func__, offset, offset + size);
+
+ return true;
+ }
+
+ return false;
+}
+
static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ int last_page = ((mtd->erasesize - mtd->writesize) >>
+chip->page_shift) & chip->pagemask;
+
if (chip->options & NAND_NO_BBM_QUIRK)
return 0;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, ofs, last_page))
+ return -EIO;
+
if (chip->legacy.block_bad)
return chip->legacy.block_bad(chip, ofs);
@@ -397,6 +436,10 @@ static int nand_do_write_oob(struct nand_chip *chip,
loff_t to,
return -EINVAL;
}
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, to, ops->ooblen))
+ return -EIO;
+
chipnr = (int)(to >> chip->chip_shift);
/*
@@ -565,6 +608,11 @@ static int nand_block_isreserved(struct mtd_info *mtd,
loff_t ofs)
if (!chip->bbt)
return 0;
+
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, ofs, 0))
+ return -EIO;
+
/* Return info from the table */
return nand_isreserved_bbt(chip, ofs);
}
@@ -3127,6 +3175,10 @@ static int nand_do_read_ops(struct nand_chip *chip,
loff_t from,
int retry_mode = 0;
bool ecc_fail = false;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, from, readlen))
+ return -EIO;
+
chipnr = (int)(from >> chip->chip_shift);
nand_select_target(chip, chipnr);
@@ -3458,6 +3510,10 @@ static int nand_do_read_oob(struct nand_chip *chip,
loff_t from,
pr_debug("%s: from = 0x%08Lx, len = %i\n",
__func__, (unsigned long long)from, readlen);
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, from, readlen))
+ return -EIO;
+
stats = mtd->ecc_stats;
len = mtd_oobavail(mtd, ops);
@@ -3979,6 +4035,10 @@ static int nand_do_write_ops(struct nand_chip *chip,
loff_t to,
return -EINVAL;
}
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, to, writelen))
+ return -EIO;
+
column = to & (mtd->writesize - 1);
chipnr = (int)(to >> chip->chip_shift);
@@ -4180,6 +4240,10 @@ int nand_erase_nand(struct nand_chip *chip, struct
erase_info *instr,
if (check_offs_len(chip, instr->addr, instr->len))
return -EINVAL;
+ /* Check if the region is secured */
+ if (nand_region_is_secured(chip, instr->addr, instr->len))
+ return -EIO;
+
/* Grab the lock and see if the device is available */
Re: [PATCH v2 0/6] Add support for Actions Semi Owl socinfo
+ Matheus On Thu, Apr 01, 2021 at 01:58:05PM +0300, Cristian Ciocaltea wrote: > Hi Mani, Andreas, > > On Thu, Apr 01, 2021 at 12:49:37PM +0200, Andreas Färber wrote: > > Hi, > > > > On 01.04.21 12:27, Manivannan Sadhasivam wrote: > > > On Thu, Apr 01, 2021 at 12:40:41PM +0300, Cristian Ciocaltea wrote: > > >> On Thu, Apr 01, 2021 at 10:54:38AM +0530, Manivannan Sadhasivam wrote: > > >>> On Tue, Mar 30, 2021 at 04:48:15PM +0300, Cristian Ciocaltea wrote: > > >>>> This patchset adds a socinfo driver which provides information about > > >>>> Actions Semi Owl SoCs to user space via sysfs: machine, family, soc_id, > > >>>> serial_number. > > >>>> > > >>>> Please note the serial number is currently available only for the S500 > > >>>> SoC variant. > > >>>> > > >>>> This has been tested on the S500 SoC based RoseapplePi SBC. > > >>>> > > >>> > > >>> Is this the soc_id provided by the vendor bootloader (uboot)? If so, > > >>> under > > >>> what basis it provides? I don't think the SoC has the provision for > > >>> soc_id based on HW parameters. > > >> > > >> No, the soc_id is not provided by the bootloader, or at least I couldn't > > >> identify any related implementation. Instead, I provided this via the > > >> driver itself, since I've encountered this approach in some other soc > > >> drivers as well (e.g. imx/soc-imx.c, versatile/soc-integrator.c). > > >> > > > > > > Sorry, I was referring to serial_number. Since your comment says so, can > > > you point to the corresponding code? > > > > Seconded that this needs to be better understood. If this is just a > > convention of some downstream U-Boot that's not implemented in mainline > > (and maybe not even for Guitar or Labrador? tested on RoseapplePi only), > > it might not be worth its own reserved-memory based kernel driver? > > The serial number is actually provided by the s500-bootloader for which > Actions did not provide the source code, at least it is not available > in the xapp github repo. I did not find anything related to this in > downstream U-Boot. > Hmm, then we can consider this as the firmware dependent property. But can we get consensus that this is common for all S500 SoCs? Maybe, Matheus can verify it on Labrador? I don't think adding a SOCINFO driver for a single board is a good idea. Thanks, Mani > Kind regards, > Cristi > > > Implementing a standard interface such as DMI tables or a DT property in > > mainline U-Boot may be more useful then. Is it still Mani's S900 only? > > > > Regards, > > Andreas > > > > -- > > SUSE Software Solutions Germany GmbH > > Maxfeldstr. 5, 90409 Nürnberg, Germany > > GF: Felix Imendörffer > > HRB 36809 (AG Nürnberg)
