| Message ID | 1454106914-15857-9-git-send-email-okaya@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Jan 29, 2016 at 05:35:11PM -0500, Sinan Kaya wrote: > Removing the flexibility to choose the event channel as there is no real > use case right now. We have been using the values in ACPI that match the HW > defaults. OS is reading the event-channel from the HW register now. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > .../devicetree/bindings/dma/qcom_hidma_mgmt.txt | 3 -- > drivers/dma/qcom/hidma.c | 39 +--------------------- > 2 files changed, 1 insertion(+), 41 deletions(-) Acked-by: Rob Herring <robh@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 29, 2016 at 05:35:11PM -0500, Sinan Kaya wrote: > Removing the flexibility to choose the event channel as there is no real > use case right now. We have been using the values in ACPI that match the HW > defaults. OS is reading the event-channel from the HW register now. Fold this in to the patches adding the binding and the driver. There is no reason to add this then remove it within the same series. Mark. > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > .../devicetree/bindings/dma/qcom_hidma_mgmt.txt | 3 -- > drivers/dma/qcom/hidma.c | 39 +--------------------- > 2 files changed, 1 insertion(+), 41 deletions(-) > > diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt > index e3677a5..fd5618b 100644 > --- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt > +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt > @@ -51,7 +51,6 @@ Required properties: > - reg: Addresses for the transfer and event channel > - interrupts: Should contain the event interrupt > - desc-count: Number of asynchronous requests this channel can handle > -- channel-index: The HW event channel completions will be delivered. > - iommus: required a iommu node > > Example: > @@ -75,7 +74,6 @@ Hypervisor OS configuration: > interrupts = <0 389 0>; > desc-count = <10>; > iommus = <&system_mmu>; > - channel-index = <4>; > }; > }; > > @@ -87,6 +85,5 @@ Guest OS configuration: > <0 0x5c0b0000 0x0 0x1000>; > interrupts = <0 389 0>; > desc-count = <10>; > - channel-index = <4>; > iommus = <&system_mmu>; > }; > diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c > index ac20bdb..7180367 100644 > --- a/drivers/dma/qcom/hidma.c > +++ b/drivers/dma/qcom/hidma.c > @@ -101,26 +101,6 @@ static unsigned int nr_desc_prm; > module_param(nr_desc_prm, uint, 0644); > MODULE_PARM_DESC(nr_desc_prm, "number of descriptors (default: 0)"); > > -#define HIDMA_MAX_CHANNELS 64 > -static int channel_idx[HIDMA_MAX_CHANNELS] = { > - [0 ... (HIDMA_MAX_CHANNELS - 1)] = -1 > -}; > - > -/* > - * Each DMA channel is associated with an event channel for interrupt > - * delivery. The event channel index usually comes from the firmware through > - * ACPI/DT. When a HIDMA channel is executed in the guest machine context (QEMU) > - * the device tree gets auto-generated based on the memory and IRQ resources > - * this driver uses on the host machine. Any device specific paraemeter such as > - * channel-index gets ignored by the QEMU. > - * We are using this command line parameter to pass the event channel index to > - * the guest machine. > - */ > -static unsigned int num_channel_idx; > -module_param_array_named(channel_idx, channel_idx, int, &num_channel_idx, > - 0644); > -MODULE_PARM_DESC(channel_idx, "channel index array for the notifications"); > -static atomic_t channel_ref_count; > > /* process completed descriptors */ > static void hidma_process_completed(struct hidma_chan *mchan) > @@ -592,7 +572,6 @@ static int hidma_probe(struct platform_device *pdev) > struct resource *trca_resource; > struct resource *evca_resource; > int chirq; > - int current_channel_index = atomic_read(&channel_ref_count); > void __iomem *evca; > void __iomem *trca; > int rc; > @@ -668,22 +647,7 @@ static int hidma_probe(struct platform_device *pdev) > goto dmafree; > } > > - if (current_channel_index > HIDMA_MAX_CHANNELS) { > - rc = -EINVAL; > - goto dmafree; > - } > - > - dmadev->chidx = -1; > - device_property_read_u32(&pdev->dev, "channel-index", &dmadev->chidx); > - > - /* kernel command line override for the guest machine */ > - if (channel_idx[current_channel_index] != -1) > - dmadev->chidx = channel_idx[current_channel_index]; > - > - if (dmadev->chidx == -1) { > - rc = -EINVAL; > - goto dmafree; > - } > + dmadev->chidx = readl(dmadev->dev_trca + 0x28); > > /* Set DMA mask to 64 bits. */ > rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); > @@ -724,7 +688,6 @@ static int hidma_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, dmadev); > pm_runtime_mark_last_busy(dmadev->ddev.dev); > pm_runtime_put_autosuspend(dmadev->ddev.dev); > - atomic_inc(&channel_ref_count); > return 0; > > uninit: > -- > 1.8.2.1 > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2/1/2016 10:35 AM, Mark Rutland wrote: > Fold this in to the patches adding the binding and the driver. > > There is no reason to add this then remove it within the same series. > > Mark. Will do. I was trying to keep changes to the minimum on the other patches to help the reviewers.
Hi Mark, On 2/1/2016 10:35 AM, Mark Rutland wrote: >> Removing the flexibility to choose the event channel as there is no real >> > use case right now. We have been using the values in ACPI that match the HW >> > defaults. OS is reading the event-channel from the HW register now. > Fold this in to the patches adding the binding and the driver. > > There is no reason to add this then remove it within the same series. > > Mark. > Before I post the next rev, I'd like to see if I missed something in the series when it comes to your comments. Can you please take a look and let me know? Sinan
On 2/1/2016 10:14 AM, Rob Herring wrote: > On Fri, Jan 29, 2016 at 05:35:11PM -0500, Sinan Kaya wrote: >> Removing the flexibility to choose the event channel as there is no real >> use case right now. We have been using the values in ACPI that match the HW >> defaults. OS is reading the event-channel from the HW register now. >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> --- >> .../devicetree/bindings/dma/qcom_hidma_mgmt.txt | 3 -- >> drivers/dma/qcom/hidma.c | 39 +--------------------- >> 2 files changed, 1 insertion(+), 41 deletions(-) > > Acked-by: Rob Herring <robh@kernel.org> > Thanks Rob, I'll merge this patch to the existing documentation and code on the next rev.
On Wed, Feb 03, 2016 at 01:32:01PM -0500, Sinan Kaya wrote: > Hi Mark, > > On 2/1/2016 10:35 AM, Mark Rutland wrote: > >> Removing the flexibility to choose the event channel as there is no real > >> > use case right now. We have been using the values in ACPI that match the HW > >> > defaults. OS is reading the event-channel from the HW register now. > > Fold this in to the patches adding the binding and the driver. > > > > There is no reason to add this then remove it within the same series. > > > > Mark. > > > > Before I post the next rev, I'd like to see if I missed something in the series when > it comes to your comments. > > Can you please take a look and let me know? Sorry, but it's going to be far easier to review with useless code removed, so I will wait until the next posting where this patch is folded in. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2/3/2016 1:36 PM, Mark Rutland wrote: >> Can you please take a look and let me know? > Sorry, but it's going to be far easier to review with useless code > removed, so I will wait until the next posting where this patch is > folded in. > > Thanks, > Mark. OK, I'll post the next rev by the end of this week.
Hi Mark, On 2/3/2016 1:36 PM, Mark Rutland wrote: > Sorry, but it's going to be far easier to review with useless code > removed, so I will wait until the next posting where this patch is > folded in. V14 of the patch have been posted on the mail-list where this patch has been folder into the existing code. https://lkml.org/lkml/2016/2/4/1086 I have also captured the information you previously asked into the cover letter as you asked. I'm looking forward to hear your other comments. Sinan
On 2/7/2016 10:04 AM, Sinan Kaya wrote: > Hi Mark, > > On 2/3/2016 1:36 PM, Mark Rutland wrote: >> > Sorry, but it's going to be far easier to review with useless code >> > removed, so I will wait until the next posting where this patch is >> > folded in. > V14 of the patch have been posted on the mail-list where this patch has > been folder into the existing code. > > https://lkml.org/lkml/2016/2/4/1086 > > I have also captured the information you previously asked into the cover > letter as you asked. > > I'm looking forward to hear your other comments. > Sinan ping
diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt index e3677a5..fd5618b 100644 --- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt @@ -51,7 +51,6 @@ Required properties: - reg: Addresses for the transfer and event channel - interrupts: Should contain the event interrupt - desc-count: Number of asynchronous requests this channel can handle -- channel-index: The HW event channel completions will be delivered. - iommus: required a iommu node Example: @@ -75,7 +74,6 @@ Hypervisor OS configuration: interrupts = <0 389 0>; desc-count = <10>; iommus = <&system_mmu>; - channel-index = <4>; }; }; @@ -87,6 +85,5 @@ Guest OS configuration: <0 0x5c0b0000 0x0 0x1000>; interrupts = <0 389 0>; desc-count = <10>; - channel-index = <4>; iommus = <&system_mmu>; }; diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c index ac20bdb..7180367 100644 --- a/drivers/dma/qcom/hidma.c +++ b/drivers/dma/qcom/hidma.c @@ -101,26 +101,6 @@ static unsigned int nr_desc_prm; module_param(nr_desc_prm, uint, 0644); MODULE_PARM_DESC(nr_desc_prm, "number of descriptors (default: 0)"); -#define HIDMA_MAX_CHANNELS 64 -static int channel_idx[HIDMA_MAX_CHANNELS] = { - [0 ... (HIDMA_MAX_CHANNELS - 1)] = -1 -}; - -/* - * Each DMA channel is associated with an event channel for interrupt - * delivery. The event channel index usually comes from the firmware through - * ACPI/DT. When a HIDMA channel is executed in the guest machine context (QEMU) - * the device tree gets auto-generated based on the memory and IRQ resources - * this driver uses on the host machine. Any device specific paraemeter such as - * channel-index gets ignored by the QEMU. - * We are using this command line parameter to pass the event channel index to - * the guest machine. - */ -static unsigned int num_channel_idx; -module_param_array_named(channel_idx, channel_idx, int, &num_channel_idx, - 0644); -MODULE_PARM_DESC(channel_idx, "channel index array for the notifications"); -static atomic_t channel_ref_count; /* process completed descriptors */ static void hidma_process_completed(struct hidma_chan *mchan) @@ -592,7 +572,6 @@ static int hidma_probe(struct platform_device *pdev) struct resource *trca_resource; struct resource *evca_resource; int chirq; - int current_channel_index = atomic_read(&channel_ref_count); void __iomem *evca; void __iomem *trca; int rc; @@ -668,22 +647,7 @@ static int hidma_probe(struct platform_device *pdev) goto dmafree; } - if (current_channel_index > HIDMA_MAX_CHANNELS) { - rc = -EINVAL; - goto dmafree; - } - - dmadev->chidx = -1; - device_property_read_u32(&pdev->dev, "channel-index", &dmadev->chidx); - - /* kernel command line override for the guest machine */ - if (channel_idx[current_channel_index] != -1) - dmadev->chidx = channel_idx[current_channel_index]; - - if (dmadev->chidx == -1) { - rc = -EINVAL; - goto dmafree; - } + dmadev->chidx = readl(dmadev->dev_trca + 0x28); /* Set DMA mask to 64 bits. */ rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); @@ -724,7 +688,6 @@ static int hidma_probe(struct platform_device *pdev) platform_set_drvdata(pdev, dmadev); pm_runtime_mark_last_busy(dmadev->ddev.dev); pm_runtime_put_autosuspend(dmadev->ddev.dev); - atomic_inc(&channel_ref_count); return 0; uninit:
Removing the flexibility to choose the event channel as there is no real use case right now. We have been using the values in ACPI that match the HW defaults. OS is reading the event-channel from the HW register now. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- .../devicetree/bindings/dma/qcom_hidma_mgmt.txt | 3 -- drivers/dma/qcom/hidma.c | 39 +--------------------- 2 files changed, 1 insertion(+), 41 deletions(-)