RE: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
> -Original Message- > From: Jens Axboe [mailto:ax...@kernel.dk] > Sent: Monday, September 23, 2013 4:43 AM > To: Olof Johansson > Cc: Andrew Morton; Jeff Moyer; OS Engineering; linux- > ker...@vger.kernel.org; Akhil Bhansali; Ramprasad Chinthekindi; Amit > Phansalkar > Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card. > > On Sun, Sep 22 2013, Olof Johansson wrote: > > On Sat, Sep 21, 2013 at 1:01 PM, Jens Axboe wrote: > > > On Fri, Sep 20 2013, Andrew Morton wrote: > > >> On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe > wrote: > > >> > > >> > > So, it looks like this driver needs a bunch of work before it's > > >> > > ready to go in. Or, maybe it's better to submit it with a TODO > > >> > > list for the staging tree instead? > > >> > > > >> > Not disagreeing with you, it definitely needs a bit of cleaning. > > >> > And so far stec has not been very responsive in fixing those issues. > > >> > > >> Geeze. Here's what I came up with, mainly to make i386 compile: > > > > > > Thanks Andrew. It's not the fixes themselves that are tricky, I > > > would just have liked a little more proof that the submission wasn't > > > just a dump'n run. > > > > Well, that's what much of the staging process is about, since it needs > > to come with a todo list (and that list needs to be taken care of > > before the driver graduates). Honestly, this looks like a prime > > candidate for that. But it's your tree and your call. > > > > Thanks for applying the patch from Andrew, that resolves most of it > > for me right now. > > I'm sort-of on the fence. It's minor stuff, but it does need cleaning up. So > not > necessarily something that will prevent it from going into 3.12, it's more the > continued support and development I'm worried about. > Hi Jens, My name is Amit and I'm managing the host drivers team at sTec (now HGST). Thank you for your continued support and feedback in this effort. We are really committed to support this driver as part of Linux open source/kernel. I'd like to assure you that this is not a dump and run submission. In the recent past, we were working with Jeff to get this driver ready for open source and have been making necessary revisions. Even in future, my team would be available to investigate the issues and patch the driver accordingly. It would be great if you could consider this driver for 3.12 inclusion. Thanks and regards, Amit Phansalkar > -- > Jens Axboe PROPRIETARY-CONFIDENTIAL INFORMATION INCLUDED This electronic transmission, and any documents attached hereto, may contain confidential, proprietary and/or legally privileged information. The information is intended only for use by the recipient named above. If you received this electronic message in error, please notify the sender and delete the electronic message. Any disclosure, copying, distribution, or use of the contents of information received in error is strictly prohibited, and violators will be pursued legally. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
-Original Message- From: Jens Axboe [mailto:ax...@kernel.dk] Sent: Monday, September 23, 2013 4:43 AM To: Olof Johansson Cc: Andrew Morton; Jeff Moyer; OS Engineering; linux- ker...@vger.kernel.org; Akhil Bhansali; Ramprasad Chinthekindi; Amit Phansalkar Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card. On Sun, Sep 22 2013, Olof Johansson wrote: On Sat, Sep 21, 2013 at 1:01 PM, Jens Axboe ax...@kernel.dk wrote: On Fri, Sep 20 2013, Andrew Morton wrote: On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe ax...@kernel.dk wrote: So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. Geeze. Here's what I came up with, mainly to make i386 compile: Thanks Andrew. It's not the fixes themselves that are tricky, I would just have liked a little more proof that the submission wasn't just a dump'n run. Well, that's what much of the staging process is about, since it needs to come with a todo list (and that list needs to be taken care of before the driver graduates). Honestly, this looks like a prime candidate for that. But it's your tree and your call. Thanks for applying the patch from Andrew, that resolves most of it for me right now. I'm sort-of on the fence. It's minor stuff, but it does need cleaning up. So not necessarily something that will prevent it from going into 3.12, it's more the continued support and development I'm worried about. Hi Jens, My name is Amit and I'm managing the host drivers team at sTec (now HGST). Thank you for your continued support and feedback in this effort. We are really committed to support this driver as part of Linux open source/kernel. I'd like to assure you that this is not a dump and run submission. In the recent past, we were working with Jeff to get this driver ready for open source and have been making necessary revisions. Even in future, my team would be available to investigate the issues and patch the driver accordingly. It would be great if you could consider this driver for 3.12 inclusion. Thanks and regards, Amit Phansalkar -- Jens Axboe PROPRIETARY-CONFIDENTIAL INFORMATION INCLUDED This electronic transmission, and any documents attached hereto, may contain confidential, proprietary and/or legally privileged information. The information is intended only for use by the recipient named above. If you received this electronic message in error, please notify the sender and delete the electronic message. Any disclosure, copying, distribution, or use of the contents of information received in error is strictly prohibited, and violators will be pursued legally. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Sun, Sep 22 2013, Olof Johansson wrote: > On Sat, Sep 21, 2013 at 1:01 PM, Jens Axboe wrote: > > On Fri, Sep 20 2013, Andrew Morton wrote: > >> On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe wrote: > >> > >> > > So, it looks like this driver needs a bunch of work before it's ready > >> > > to go in. Or, maybe it's better to submit it with a TODO list for the > >> > > staging tree instead? > >> > > >> > Not disagreeing with you, it definitely needs a bit of cleaning. And so > >> > far stec has not been very responsive in fixing those issues. > >> > >> Geeze. Here's what I came up with, mainly to make i386 compile: > > > > Thanks Andrew. It's not the fixes themselves that are tricky, I would > > just have liked a little more proof that the submission wasn't just a > > dump'n run. > > Well, that's what much of the staging process is about, since it needs > to come with a todo list (and that list needs to be taken care of > before the driver graduates). Honestly, this looks like a prime > candidate for that. But it's your tree and your call. > > Thanks for applying the patch from Andrew, that resolves most of it > for me right now. I'm sort-of on the fence. It's minor stuff, but it does need cleaning up. So not necessarily something that will prevent it from going into 3.12, it's more the continued support and development I'm worried about. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Sat, Sep 21, 2013 at 1:01 PM, Jens Axboe wrote: > On Fri, Sep 20 2013, Andrew Morton wrote: >> On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe wrote: >> >> > > So, it looks like this driver needs a bunch of work before it's ready >> > > to go in. Or, maybe it's better to submit it with a TODO list for the >> > > staging tree instead? >> > >> > Not disagreeing with you, it definitely needs a bit of cleaning. And so >> > far stec has not been very responsive in fixing those issues. >> >> Geeze. Here's what I came up with, mainly to make i386 compile: > > Thanks Andrew. It's not the fixes themselves that are tricky, I would > just have liked a little more proof that the submission wasn't just a > dump'n run. Well, that's what much of the staging process is about, since it needs to come with a todo list (and that list needs to be taken care of before the driver graduates). Honestly, this looks like a prime candidate for that. But it's your tree and your call. Thanks for applying the patch from Andrew, that resolves most of it for me right now. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Sat, Sep 21, 2013 at 1:01 PM, Jens Axboe ax...@kernel.dk wrote: On Fri, Sep 20 2013, Andrew Morton wrote: On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe ax...@kernel.dk wrote: So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. Geeze. Here's what I came up with, mainly to make i386 compile: Thanks Andrew. It's not the fixes themselves that are tricky, I would just have liked a little more proof that the submission wasn't just a dump'n run. Well, that's what much of the staging process is about, since it needs to come with a todo list (and that list needs to be taken care of before the driver graduates). Honestly, this looks like a prime candidate for that. But it's your tree and your call. Thanks for applying the patch from Andrew, that resolves most of it for me right now. -Olof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Sun, Sep 22 2013, Olof Johansson wrote: On Sat, Sep 21, 2013 at 1:01 PM, Jens Axboe ax...@kernel.dk wrote: On Fri, Sep 20 2013, Andrew Morton wrote: On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe ax...@kernel.dk wrote: So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. Geeze. Here's what I came up with, mainly to make i386 compile: Thanks Andrew. It's not the fixes themselves that are tricky, I would just have liked a little more proof that the submission wasn't just a dump'n run. Well, that's what much of the staging process is about, since it needs to come with a todo list (and that list needs to be taken care of before the driver graduates). Honestly, this looks like a prime candidate for that. But it's your tree and your call. Thanks for applying the patch from Andrew, that resolves most of it for me right now. I'm sort-of on the fence. It's minor stuff, but it does need cleaning up. So not necessarily something that will prevent it from going into 3.12, it's more the continued support and development I'm worried about. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Fri, Sep 20 2013, Andrew Morton wrote: > On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe wrote: > > > > So, it looks like this driver needs a bunch of work before it's ready > > > to go in. Or, maybe it's better to submit it with a TODO list for the > > > staging tree instead? > > > > Not disagreeing with you, it definitely needs a bit of cleaning. And so > > far stec has not been very responsive in fixing those issues. > > Geeze. Here's what I came up with, mainly to make i386 compile: Thanks Andrew. It's not the fixes themselves that are tricky, I would just have liked a little more proof that the submission wasn't just a dump'n run. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Fri, Sep 20 2013, Andrew Morton wrote: On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe ax...@kernel.dk wrote: So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. Geeze. Here's what I came up with, mainly to make i386 compile: Thanks Andrew. It's not the fixes themselves that are tricky, I would just have liked a little more proof that the submission wasn't just a dump'n run. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe wrote: > > So, it looks like this driver needs a bunch of work before it's ready > > to go in. Or, maybe it's better to submit it with a TODO list for the > > staging tree instead? > > Not disagreeing with you, it definitely needs a bit of cleaning. And so > far stec has not been very responsive in fixing those issues. Geeze. Here's what I came up with, mainly to make i386 compile: From: Andrew Morton Subject: drivers/block/skd_main.c: fix a few things, disable on 32-bit Fix these (i386 allmodconfig): drivers/block/skd_main.c:4559: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'unsigned int' drivers/block/skd_main.c:4614: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'unsigned int' drivers/block/skd_main.c:4614: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'unsigned int' drivers/block/skd_main.c:4626: warning: format '%lu' expects type 'long unsigned int', but argument 6 has type 'unsigned int' drivers/block/skd_main.c:4626: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'unsigned int' drivers/block/skd_main.c:4671: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'unsigned int' drivers/block/skd_main.c:4671: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'unsigned int' And what remains is these: drivers/block/skd_main.c: In function 'skd_reg_write64': drivers/block/skd_main.c:439: error: implicit declaration of function 'writeq' drivers/block/skd_main.c:441: error: implicit declaration of function 'readq' drivers/block/skd_main.c: In function 'skd_cons_skmsg': drivers/block/skd_main.c:4589: warning: cast from pointer to integer of different size drivers/block/skd_main.c:4592: warning: cast from pointer to integer of different size drivers/block/skd_main.c:4593: warning: cast to pointer from integer of different size Which is more than I'm prepared to address, so just disable the driver on 32-bit builds. Cc: Akhil Bhansali Cc: Jeff Moyer Cc: Jens Axboe Signed-off-by: Andrew Morton --- drivers/block/Kconfig|1 + drivers/block/skd_main.c |8 2 files changed, 5 insertions(+), 4 deletions(-) diff -puN drivers/block/skd_main.c~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit drivers/block/skd_main.c --- a/drivers/block/skd_main.c~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit +++ a/drivers/block/skd_main.c @@ -4556,7 +4556,7 @@ static int skd_cons_skmsg(struct skd_dev int rc = 0; u32 i; - VPRINTK(skdev, "skmsg_table kzalloc, struct %lu, count %u total %lu\n", + VPRINTK(skdev, "skmsg_table kzalloc, struct %u, count %u total %lu\n", sizeof(struct skd_fitmsg_context), skdev->num_fitmsg_context, (unsigned long) sizeof(struct skd_fitmsg_context) * @@ -4611,7 +4611,7 @@ static int skd_cons_skreq(struct skd_dev int rc = 0; u32 i; - VPRINTK(skdev, "skreq_table kzalloc, struct %lu, count %u total %lu\n", + VPRINTK(skdev, "skreq_table kzalloc, struct %u, count %u total %u\n", sizeof(struct skd_request_context), skdev->num_req_context, sizeof(struct skd_request_context) * skdev->num_req_context); @@ -4623,7 +4623,7 @@ static int skd_cons_skreq(struct skd_dev goto err_out; } - VPRINTK(skdev, "alloc sg_table sg_per_req %u scatlist %lu total %lu\n", + VPRINTK(skdev, "alloc sg_table sg_per_req %u scatlist %u total %u\n", skdev->sgs_per_request, sizeof(struct scatterlist), skdev->sgs_per_request * sizeof(struct scatterlist)); @@ -4668,7 +4668,7 @@ static int skd_cons_skspcl(struct skd_de int rc = 0; u32 i, nbytes; - VPRINTK(skdev, "skspcl_table kzalloc, struct %lu, count %u total %lu\n", + VPRINTK(skdev, "skspcl_table kzalloc, struct %u, count %u total %u\n", sizeof(struct skd_special_context), skdev->n_special, sizeof(struct skd_special_context) * skdev->n_special); diff -puN drivers/block/Kconfig~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit drivers/block/Kconfig --- a/drivers/block/Kconfig~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit +++ a/drivers/block/Kconfig @@ -319,6 +319,7 @@ config BLK_DEV_NVME config BLK_DEV_SKD tristate "STEC S1120 Block Driver" depends on PCI + depends on 64BIT ---help--- Saying Y or M here will enable support for the STEC, Inc. S1120 PCIe SSD. _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Tue, 17 Sep 2013 14:20:55 -0600 Jens Axboe ax...@kernel.dk wrote: So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. Geeze. Here's what I came up with, mainly to make i386 compile: From: Andrew Morton a...@linux-foundation.org Subject: drivers/block/skd_main.c: fix a few things, disable on 32-bit Fix these (i386 allmodconfig): drivers/block/skd_main.c:4559: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'unsigned int' drivers/block/skd_main.c:4614: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'unsigned int' drivers/block/skd_main.c:4614: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'unsigned int' drivers/block/skd_main.c:4626: warning: format '%lu' expects type 'long unsigned int', but argument 6 has type 'unsigned int' drivers/block/skd_main.c:4626: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'unsigned int' drivers/block/skd_main.c:4671: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'unsigned int' drivers/block/skd_main.c:4671: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'unsigned int' And what remains is these: drivers/block/skd_main.c: In function 'skd_reg_write64': drivers/block/skd_main.c:439: error: implicit declaration of function 'writeq' drivers/block/skd_main.c:441: error: implicit declaration of function 'readq' drivers/block/skd_main.c: In function 'skd_cons_skmsg': drivers/block/skd_main.c:4589: warning: cast from pointer to integer of different size drivers/block/skd_main.c:4592: warning: cast from pointer to integer of different size drivers/block/skd_main.c:4593: warning: cast to pointer from integer of different size Which is more than I'm prepared to address, so just disable the driver on 32-bit builds. Cc: Akhil Bhansali abhans...@stec-inc.com Cc: Jeff Moyer jmo...@redhat.com Cc: Jens Axboe ax...@kernel.dk Signed-off-by: Andrew Morton a...@linux-foundation.org --- drivers/block/Kconfig|1 + drivers/block/skd_main.c |8 2 files changed, 5 insertions(+), 4 deletions(-) diff -puN drivers/block/skd_main.c~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit drivers/block/skd_main.c --- a/drivers/block/skd_main.c~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit +++ a/drivers/block/skd_main.c @@ -4556,7 +4556,7 @@ static int skd_cons_skmsg(struct skd_dev int rc = 0; u32 i; - VPRINTK(skdev, skmsg_table kzalloc, struct %lu, count %u total %lu\n, + VPRINTK(skdev, skmsg_table kzalloc, struct %u, count %u total %lu\n, sizeof(struct skd_fitmsg_context), skdev-num_fitmsg_context, (unsigned long) sizeof(struct skd_fitmsg_context) * @@ -4611,7 +4611,7 @@ static int skd_cons_skreq(struct skd_dev int rc = 0; u32 i; - VPRINTK(skdev, skreq_table kzalloc, struct %lu, count %u total %lu\n, + VPRINTK(skdev, skreq_table kzalloc, struct %u, count %u total %u\n, sizeof(struct skd_request_context), skdev-num_req_context, sizeof(struct skd_request_context) * skdev-num_req_context); @@ -4623,7 +4623,7 @@ static int skd_cons_skreq(struct skd_dev goto err_out; } - VPRINTK(skdev, alloc sg_table sg_per_req %u scatlist %lu total %lu\n, + VPRINTK(skdev, alloc sg_table sg_per_req %u scatlist %u total %u\n, skdev-sgs_per_request, sizeof(struct scatterlist), skdev-sgs_per_request * sizeof(struct scatterlist)); @@ -4668,7 +4668,7 @@ static int skd_cons_skspcl(struct skd_de int rc = 0; u32 i, nbytes; - VPRINTK(skdev, skspcl_table kzalloc, struct %lu, count %u total %lu\n, + VPRINTK(skdev, skspcl_table kzalloc, struct %u, count %u total %u\n, sizeof(struct skd_special_context), skdev-n_special, sizeof(struct skd_special_context) * skdev-n_special); diff -puN drivers/block/Kconfig~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit drivers/block/Kconfig --- a/drivers/block/Kconfig~drivers-block-skd_mainc-fix-a-few-things-disable-on-32-bit +++ a/drivers/block/Kconfig @@ -319,6 +319,7 @@ config BLK_DEV_NVME config BLK_DEV_SKD tristate STEC S1120 Block Driver depends on PCI + depends on 64BIT ---help--- Saying Y or M here will enable support for the STEC, Inc. S1120 PCIe SSD. _ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On 09/17/2013 01:04 PM, Olof Johansson wrote: > On Thu, Sep 5, 2013 at 7:12 AM, Jens Axboe wrote: >> On 09/05/2013 06:00 AM, Jeff Moyer wrote: >>> OS Engineering writes: >>> Hi Jeff, Thank you for reviewing the patch. >>> >>> No problem. Jens, any objection to queueing this up for 3.12? >> >> I'll give it a look-over, but usually I'm pretty lax when it comes to >> new drivers. So no, I'd be surprised if we can't queue this up for 3.12. > > I came across this driver because it was spewing a lot of really > trivial and easy to fix compiler warnings. Silly stuff such as > printing u32 with %lu. > > From a quick look at the code, several things are immediately apparent: > > First, checkpatch says, on the currently existing file in -next: > > total: 3 errors, 61 warnings, 5817 lines checked > > Code like this looks _really_ confused: > > barrier(); > val = readl(skdev->mem_map[1] + offset); > barrier(); > > There are also some crazy long functions that should be refactored, > such as skd_request_fn(). > > So, it looks like this driver needs a bunch of work before it's ready > to go in. Or, maybe it's better to submit it with a TODO list for the > staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Thu, Sep 5, 2013 at 7:12 AM, Jens Axboe wrote: > On 09/05/2013 06:00 AM, Jeff Moyer wrote: >> OS Engineering writes: >> >>> Hi Jeff, >>> >>> Thank you for reviewing the patch. >> >> No problem. Jens, any objection to queueing this up for 3.12? > > I'll give it a look-over, but usually I'm pretty lax when it comes to > new drivers. So no, I'd be surprised if we can't queue this up for 3.12. I came across this driver because it was spewing a lot of really trivial and easy to fix compiler warnings. Silly stuff such as printing u32 with %lu. >From a quick look at the code, several things are immediately apparent: First, checkpatch says, on the currently existing file in -next: total: 3 errors, 61 warnings, 5817 lines checked Code like this looks _really_ confused: barrier(); val = readl(skdev->mem_map[1] + offset); barrier(); There are also some crazy long functions that should be refactored, such as skd_request_fn(). So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On Thu, Sep 5, 2013 at 7:12 AM, Jens Axboe ax...@kernel.dk wrote: On 09/05/2013 06:00 AM, Jeff Moyer wrote: OS Engineering osengineer...@stec-inc.com writes: Hi Jeff, Thank you for reviewing the patch. No problem. Jens, any objection to queueing this up for 3.12? I'll give it a look-over, but usually I'm pretty lax when it comes to new drivers. So no, I'd be surprised if we can't queue this up for 3.12. I came across this driver because it was spewing a lot of really trivial and easy to fix compiler warnings. Silly stuff such as printing u32 with %lu. From a quick look at the code, several things are immediately apparent: First, checkpatch says, on the currently existing file in -next: total: 3 errors, 61 warnings, 5817 lines checked Code like this looks _really_ confused: barrier(); val = readl(skdev-mem_map[1] + offset); barrier(); There are also some crazy long functions that should be refactored, such as skd_request_fn(). So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? -Olof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On 09/17/2013 01:04 PM, Olof Johansson wrote: On Thu, Sep 5, 2013 at 7:12 AM, Jens Axboe ax...@kernel.dk wrote: On 09/05/2013 06:00 AM, Jeff Moyer wrote: OS Engineering osengineer...@stec-inc.com writes: Hi Jeff, Thank you for reviewing the patch. No problem. Jens, any objection to queueing this up for 3.12? I'll give it a look-over, but usually I'm pretty lax when it comes to new drivers. So no, I'd be surprised if we can't queue this up for 3.12. I came across this driver because it was spewing a lot of really trivial and easy to fix compiler warnings. Silly stuff such as printing u32 with %lu. From a quick look at the code, several things are immediately apparent: First, checkpatch says, on the currently existing file in -next: total: 3 errors, 61 warnings, 5817 lines checked Code like this looks _really_ confused: barrier(); val = readl(skdev-mem_map[1] + offset); barrier(); There are also some crazy long functions that should be refactored, such as skd_request_fn(). So, it looks like this driver needs a bunch of work before it's ready to go in. Or, maybe it's better to submit it with a TODO list for the staging tree instead? Not disagreeing with you, it definitely needs a bit of cleaning. And so far stec has not been very responsive in fixing those issues. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On 09/05/2013 06:00 AM, Jeff Moyer wrote: > OS Engineering writes: > >> Hi Jeff, >> >> Thank you for reviewing the patch. > > No problem. Jens, any objection to queueing this up for 3.12? I'll give it a look-over, but usually I'm pretty lax when it comes to new drivers. So no, I'd be surprised if we can't queue this up for 3.12. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
OS Engineering writes: > Hi Jeff, > > Thank you for reviewing the patch. No problem. Jens, any objection to queueing this up for 3.12? Cheers, Jeff > > From: Jeff Moyer [jmo...@redhat.com] > Sent: Friday, August 30, 2013 6:18 AM > To: OS Engineering > Cc: linux-kernel@vger.kernel.org; ax...@kernel.dk; Akhil Bhansali; Ramprasad > Chinthekindi; Amit Phansalkar > Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card. > > OS Engineering writes: > >> Hello All, >> >> We are submitting device driver for sTec's PCIe flash card named "Kronos" >> for inclusion in linux kernel. >> >> Thanks. >> Akhil Bhansali. >> -- >> From 3861f12cd99a9b1ba561296790ec482a75eeacd1 Mon Sep 17 00:00:00 2001 >> From: Akhil Bhansali >> Date: Fri, 30 Aug 2013 11:32:49 +0530 >> Subject: [PATCH] Patch for pcie device driver from sTec Inc. >> >> This patch adds "skd", a driver for sTec's PCIe flash cards named >> Kronos. >> Signed-off-by: Akhil Bhansali >> Signed-off-by: Ramprasad Chinthekindi > > This looks good to me. It's nice to see another open source PCIe SSD > driver submitted! > > Reviewed-by: Jeff Moyer > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
OS Engineering osengineer...@stec-inc.com writes: Hi Jeff, Thank you for reviewing the patch. No problem. Jens, any objection to queueing this up for 3.12? Cheers, Jeff From: Jeff Moyer [jmo...@redhat.com] Sent: Friday, August 30, 2013 6:18 AM To: OS Engineering Cc: linux-kernel@vger.kernel.org; ax...@kernel.dk; Akhil Bhansali; Ramprasad Chinthekindi; Amit Phansalkar Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card. OS Engineering osengineer...@stec-inc.com writes: Hello All, We are submitting device driver for sTec's PCIe flash card named Kronos for inclusion in linux kernel. Thanks. Akhil Bhansali. -- From 3861f12cd99a9b1ba561296790ec482a75eeacd1 Mon Sep 17 00:00:00 2001 From: Akhil Bhansali abhans...@stec-inc.com Date: Fri, 30 Aug 2013 11:32:49 +0530 Subject: [PATCH] Patch for pcie device driver from sTec Inc. This patch adds skd, a driver for sTec's PCIe flash cards named Kronos. Signed-off-by: Akhil Bhansali abhans...@stec-inc.com Signed-off-by: Ramprasad Chinthekindi rchintheki...@stec-inc.com This looks good to me. It's nice to see another open source PCIe SSD driver submitted! Reviewed-by: Jeff Moyer jmo...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
On 09/05/2013 06:00 AM, Jeff Moyer wrote: OS Engineering osengineer...@stec-inc.com writes: Hi Jeff, Thank you for reviewing the patch. No problem. Jens, any objection to queueing this up for 3.12? I'll give it a look-over, but usually I'm pretty lax when it comes to new drivers. So no, I'd be surprised if we can't queue this up for 3.12. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
Hi Jeff, Thank you for reviewing the patch. Regards, Akhil From: Jeff Moyer [jmo...@redhat.com] Sent: Friday, August 30, 2013 6:18 AM To: OS Engineering Cc: linux-kernel@vger.kernel.org; ax...@kernel.dk; Akhil Bhansali; Ramprasad Chinthekindi; Amit Phansalkar Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card. OS Engineering writes: > Hello All, > > We are submitting device driver for sTec's PCIe flash card named "Kronos" for > inclusion in linux kernel. > > Thanks. > Akhil Bhansali. > -- > From 3861f12cd99a9b1ba561296790ec482a75eeacd1 Mon Sep 17 00:00:00 2001 > From: Akhil Bhansali > Date: Fri, 30 Aug 2013 11:32:49 +0530 > Subject: [PATCH] Patch for pcie device driver from sTec Inc. > > This patch adds "skd", a driver for sTec's PCIe flash cards named > Kronos. > Signed-off-by: Akhil Bhansali > Signed-off-by: Ramprasad Chinthekindi This looks good to me. It's nice to see another open source PCIe SSD driver submitted! Reviewed-by: Jeff Moyer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
Hi Jeff, Thank you for reviewing the patch. Regards, Akhil From: Jeff Moyer [jmo...@redhat.com] Sent: Friday, August 30, 2013 6:18 AM To: OS Engineering Cc: linux-kernel@vger.kernel.org; ax...@kernel.dk; Akhil Bhansali; Ramprasad Chinthekindi; Amit Phansalkar Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card. OS Engineering osengineer...@stec-inc.com writes: Hello All, We are submitting device driver for sTec's PCIe flash card named Kronos for inclusion in linux kernel. Thanks. Akhil Bhansali. -- From 3861f12cd99a9b1ba561296790ec482a75eeacd1 Mon Sep 17 00:00:00 2001 From: Akhil Bhansali abhans...@stec-inc.com Date: Fri, 30 Aug 2013 11:32:49 +0530 Subject: [PATCH] Patch for pcie device driver from sTec Inc. This patch adds skd, a driver for sTec's PCIe flash cards named Kronos. Signed-off-by: Akhil Bhansali abhans...@stec-inc.com Signed-off-by: Ramprasad Chinthekindi rchintheki...@stec-inc.com This looks good to me. It's nice to see another open source PCIe SSD driver submitted! Reviewed-by: Jeff Moyer jmo...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
OS Engineering writes: > Hello All, > > We are submitting device driver for sTec's PCIe flash card named "Kronos" for > inclusion in linux kernel. > > Thanks. > Akhil Bhansali. > -- > From 3861f12cd99a9b1ba561296790ec482a75eeacd1 Mon Sep 17 00:00:00 2001 > From: Akhil Bhansali > Date: Fri, 30 Aug 2013 11:32:49 +0530 > Subject: [PATCH] Patch for pcie device driver from sTec Inc. > > This patch adds "skd", a driver for sTec's PCIe flash cards named > Kronos. > Signed-off-by: Akhil Bhansali > Signed-off-by: Ramprasad Chinthekindi This looks good to me. It's nice to see another open source PCIe SSD driver submitted! Reviewed-by: Jeff Moyer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
OS Engineering osengineer...@stec-inc.com writes: Hello All, We are submitting device driver for sTec's PCIe flash card named Kronos for inclusion in linux kernel. Thanks. Akhil Bhansali. -- From 3861f12cd99a9b1ba561296790ec482a75eeacd1 Mon Sep 17 00:00:00 2001 From: Akhil Bhansali abhans...@stec-inc.com Date: Fri, 30 Aug 2013 11:32:49 +0530 Subject: [PATCH] Patch for pcie device driver from sTec Inc. This patch adds skd, a driver for sTec's PCIe flash cards named Kronos. Signed-off-by: Akhil Bhansali abhans...@stec-inc.com Signed-off-by: Ramprasad Chinthekindi rchintheki...@stec-inc.com This looks good to me. It's nice to see another open source PCIe SSD driver submitted! Reviewed-by: Jeff Moyer jmo...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
