Re: [PATCH 2/2] arcmsr: adds code for support areca new adapter ARC1203
On Tue, 2015-11-24 at 02:24 -0800, Joe Perches wrote:
> On Tue, 2015-11-24 at 17:53 +0800, Ching Huang wrote:
> > On Tue, 2015-11-24 at 01:33 -0800, Joe Perches wrote:
> > > On Tue, 2015-11-24 at 16:17 +0800, Ching Huang wrote:
> > > > From: Ching Huang
> > > >
> > > > Support areca new PCIe to SATA RAID adapter ARC1203
> > >
> > > Why add the dma_free_coherent to an old data path?
> > > Is that a general bug fix that should be backported?
> >
> > That's right. It's need to release the allocated resource for failed
> > condition.
>
> Then the dma_free_coherent addition should be a separate patch.
>
> Style trivia:
>
> The goto to another error path like that is odd and
> the label is unintelligible.
>
> Ideally error condition handling would use a goto and
> a separate and obviously named label. Use multiple
> labels for cases with more complicated unwinding.
>
> Dan Carpenter has written about this several times.
>
> For this use, something like:
>
> writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell);
> if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> logging_message(...);
> goto
> err_free_resource;
> }
> writel(ARCMSR_MESSAGE_GET_CONFIG, reg->drv2iop_doorbell);
> if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> logging_message(...);
> goto err_free_resource;
> }
>
> [success path...]
>
> return true;
>
> err_free_resource:
> dma_free_coherent(...);
>
> return false;
> }
>
Thanks for Joe's advice.
Revised as below.
Signed-of-by: Ching Huang
---
diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-24 11:36:20.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h 2015-10-19 15:57:08.0 +0800
@@ -74,6 +74,9 @@ struct device_attribute;
#ifndef PCI_DEVICE_ID_ARECA_1214
#define PCI_DEVICE_ID_ARECA_12140x1214
#endif
+#ifndef PCI_DEVICE_ID_ARECA_1203
+ #define PCI_DEVICE_ID_ARECA_12030x1203
+#endif
/*
**
**
@@ -245,6 +248,12 @@ struct FIRMWARE_INFO
/* window of "instruction flags" from iop to driver */
#define ARCMSR_IOP2DRV_DOORBELL 0x00020408
#define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
+/* window of "instruction flags" from iop to driver */
+#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
+#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
+/* window of "instruction flags" from driver to iop */
+#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
+#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
/* ARECA FLAG LANGUAGE */
/* ioctl transfer */
#define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0
+0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 18:58:40.640226000
+0800
@@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
static const char *arcmsr_info(struct Scsi_Host *);
static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
static void arcmsr_free_irq(struct pci_dev *, struct
AdapterControlBlock *);
+static void arcmsr_wait_firmware_ready(struct AdapterControlBlock
*acb);
static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
queue_depth)
{
if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
@@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
.driver_data = ACB_ADAPTER_TYPE_B},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
.driver_data = ACB_ADAPTER_TYPE_B},
+ {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
+ .driver_data = ACB_ADAPTER_TYPE_B},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
.driver_data = ACB_ADAPTER_TYPE_A},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
@@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
}
static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
{
- struct MessageUnit_B *reg = acb->pmuB;
+ struct MessageUnit_B *reg;
struct pci_dev *pdev = acb->pdev;
void *dma_coherent;
dma_addr_t dma_coherent_handle;
@@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
acb->dma_coherent2 = dma_coherent;
reg = (struct MessageUnit_B *)dma_coherent;
acb->pmuB = reg;
- reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
- reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
- reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
-
Re: [PATCH 2/2] arcmsr: adds code for support areca new adapter ARC1203
On Tue, 2015-11-24 at 17:53 +0800, Ching Huang wrote:
> On Tue, 2015-11-24 at 01:33 -0800, Joe Perches wrote:
> > On Tue, 2015-11-24 at 16:17 +0800, Ching Huang wrote:
> > > From: Ching Huang
> > >
> > > Support areca new PCIe to SATA RAID adapter ARC1203
> >
> > Why add the dma_free_coherent to an old data path?
> > Is that a general bug fix that should be backported?
>
> That's right. It's need to release the allocated resource for failed
> condition.
Then the dma_free_coherent addition should be a separate patch.
Style trivia:
The goto to another error path like that is odd and
the label is unintelligible.
Ideally error condition handling would use a goto and
a separate and obviously named label. Use multiple
labels for cases with more complicated unwinding.
Dan Carpenter has written about this several times.
For this use, something like:
writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell);
if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
logging_message(...);
goto
err_free_resource;
}
writel(ARCMSR_MESSAGE_GET_CONFIG, reg->drv2iop_doorbell);
if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
logging_message(...);
goto err_free_resource;
}
[success path...]
return true;
err_free_resource:
dma_free_coherent(...);
return false;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arcmsr: adds code for support areca new adapter ARC1203
On Tue, 2015-11-24 at 01:33 -0800, Joe Perches wrote:
> On Tue, 2015-11-24 at 16:17 +0800, Ching Huang wrote:
> > From: Ching Huang
> >
> > Support areca new PCIe to SATA RAID adapter ARC1203
>
> Why add the dma_free_coherent to an old data path?
> Is that a general bug fix that should be backported?
That's right. It's need to release the allocated resource for failed condition.
>
> btw: the printk above the dma_free_coherent use
> shouldn't use a \ line continuation.
> it adds undesired whitespace to the format.
>
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> > b/drivers/scsi/arcmsr/arcmsr_hba.c
> []
> > @@ -2660,10 +2670,19 @@ static bool arcmsr_hbaB_get_config(struc
> > iop_firm_version = (char __iomem *)(®->message_rwbuffer[17]);> >
> > /*firm_version,17,68-83*/
> > iop_device_map = (char __iomem *)(®->message_rwbuffer[21]);>>
> > /*firm_version,21,84-99*/
> >
> > + arcmsr_wait_firmware_ready(acb);
> > + writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell);
> > + if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> > + printk(KERN_ERR "arcmsr%d: can't set driver mode.\n",
> > acb->host->host_no);
> > + goto gcfg1;
> > + }
> > writel(ARCMSR_MESSAGE_GET_CONFIG, reg->drv2iop_doorbell);
> > if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> > printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
> > miscellaneous data' timeout \n", acb->host->host_no);
> > +gcfg1:
> > + dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
> > + acb->dma_coherent2, acb->dma_coherent_handle2);
> > return false;
> > }
> > count = 8;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] arcmsr: adds code for support areca new adapter ARC1203
On Tue, 2015-11-24 at 16:17 +0800, Ching Huang wrote:
> From: Ching Huang
>
> Support areca new PCIe to SATA RAID adapter ARC1203
Why add the dma_free_coherent to an old data path?
Is that a general bug fix that should be backported?
btw: the printk above the dma_free_coherent use
shouldn't use a \ line continuation.
it adds undesired whitespace to the format.
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
[]
> @@ -2660,10 +2670,19 @@ static bool arcmsr_hbaB_get_config(struc
> iop_firm_version = (char __iomem *)(®->message_rwbuffer[17]);> >
> /*firm_version,17,68-83*/
> iop_device_map = (char __iomem *)(®->message_rwbuffer[21]);>>
> /*firm_version,21,84-99*/
>
> + arcmsr_wait_firmware_ready(acb);
> + writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell);
> + if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> + printk(KERN_ERR "arcmsr%d: can't set driver mode.\n",
> acb->host->host_no);
> + goto gcfg1;
> + }
> writel(ARCMSR_MESSAGE_GET_CONFIG, reg->drv2iop_doorbell);
> if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
> miscellaneous data' timeout \n", acb->host->host_no);
> +gcfg1:
> + dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
> + acb->dma_coherent2, acb->dma_coherent_handle2);
> return false;
> }
> count = 8;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] arcmsr: adds code for support areca new adapter ARC1203
From: Ching Huang
Support areca new PCIe to SATA RAID adapter ARC1203
Signed-of-by: Ching Huang
---
diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-24 11:36:20.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h 2015-10-19 15:57:08.0 +0800
@@ -74,6 +74,9 @@ struct device_attribute;
#ifndef PCI_DEVICE_ID_ARECA_1214
#define PCI_DEVICE_ID_ARECA_12140x1214
#endif
+#ifndef PCI_DEVICE_ID_ARECA_1203
+ #define PCI_DEVICE_ID_ARECA_12030x1203
+#endif
/*
**
**
@@ -245,6 +248,12 @@ struct FIRMWARE_INFO
/* window of "instruction flags" from iop to driver */
#define ARCMSR_IOP2DRV_DOORBELL 0x00020408
#define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
+/* window of "instruction flags" from iop to driver */
+#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
+#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
+/* window of "instruction flags" from driver to iop */
+#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
+#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
/* ARECA FLAG LANGUAGE */
/* ioctl transfer */
#define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-23 18:03:30.0 +0800
@@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
static const char *arcmsr_info(struct Scsi_Host *);
static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
static void arcmsr_free_irq(struct pci_dev *, struct AdapterControlBlock *);
+static void arcmsr_wait_firmware_ready(struct AdapterControlBlock *acb);
static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
queue_depth)
{
if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
@@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
.driver_data = ACB_ADAPTER_TYPE_B},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
.driver_data = ACB_ADAPTER_TYPE_B},
+ {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
+ .driver_data = ACB_ADAPTER_TYPE_B},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
.driver_data = ACB_ADAPTER_TYPE_A},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
@@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
}
static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
{
- struct MessageUnit_B *reg = acb->pmuB;
+ struct MessageUnit_B *reg;
struct pci_dev *pdev = acb->pdev;
void *dma_coherent;
dma_addr_t dma_coherent_handle;
@@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
acb->dma_coherent2 = dma_coherent;
reg = (struct MessageUnit_B *)dma_coherent;
acb->pmuB = reg;
- reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
- reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
- reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
- reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK);
+ if (acb->pdev->device == PCI_DEVICE_ID_ARECA_1203) {
+ reg->drv2iop_doorbell = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_1203);
+ reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK_1203);
+ reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_1203);
+ reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK_1203);
+ } else {
+ reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
+ reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
+ reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
+ reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK);
+ }
reg->message_wbuffer = (uint32_t __iomem *)((unsigned
long)acb->mem_base1 + ARCMSR_MESSAGE_WBUFFER);
reg->message_rbuffer = (uint32_t __iomem *)((unsigned
long)acb->mem_base1 + ARCMSR_MESSAGE_RBUFFER);
reg->message_rwbuffer = (uint32_t __iomem *)((unsi
