Re: [PATCH v4 WIP 1/4] parport: add device-model to parport subsystem

From: Jean Delvare
Date: Sat May 02 2015 - 10:21:07 EST


Hi Sudip,

On Tue, 28 Apr 2015 17:00:20 +0530, Sudip Mukherjee wrote:
> another WIP for your review. since this is not a formal patch for
> applying so writing the comments here.

You should still provide a proper description as if the patch was ready
to be committed. Ultimately the descriptions are going to be part of
the commits, so they need to be reviewed too.

The history is good to have too for now, but it should go after the
"---" separator, as it won't be part of the commit.

> v4: use of is_parport() is introduced to check the type of device that
> has been passed to probe or match_port.
>
> v3: started use of parport_del_port(). previously it was creating some
> ghost parallel ports during port probing. parport_del_port() removes
> registered ports if probing has failed.
>
> v2 started using probe function. Without probe, whenever any driver is
> trying to register, it is getting bound to all the available parallel
> ports. To solve that probe was required.
> Now the driver is binding only to the device it has registered. And
> that device will appear as a subdevice of the particular parallel port
> it wants to use.
>
> v2 had one more problem: it was creating some ghost parallel ports
> during port probing. from v3 we have the use of parport_del_port
> to remove registerd ports if probing has failed.

Spelling: "registered".

(As pointed out by ./scripts/checkpatch.pl - did you run it on each
patch?)

>
> In v1 Greg mentioned that we do not need to maintain our own list. That
> has been partially done. we are no longer maintaining the list of
> drivers. But we still need to maintain the list of ports and devices as
> that will be used by drivers which are not yet converted to device
> model. When all drivers are converted to use the device-model parallel
> port all these lists can be removed.
>
> Signed-off-by: Sudip Mukherjee <sudip@xxxxxxxxxxxxxxx>
> ---
> drivers/parport/parport_pc.c | 4 +-
> drivers/parport/procfs.c | 15 ++-
> drivers/parport/share.c | 266 ++++++++++++++++++++++++++++++++++++++++---
> include/linux/parport.h | 41 ++++++-
> 4 files changed, 308 insertions(+), 18 deletions(-)
> (...)

Patch tested, no functional regression found.

Tested-by: Jean Delvare <jdelvare@xxxxxxx>

--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/