lkml.org 
[lkml]   [2007]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    SubjectRe: [patch/rfc 2.6.20-git] parport reports physical devices
    Hi David,

    On Mon, 19 Feb 2007 08:40:30 -0800, David Brownell wrote:
    > On Monday 19 February 2007 6:18 am, Jean Delvare wrote:
    > > Hi David,
    > >
    > > On Sun, 18 Feb 2007 21:08:07 -0800, David Brownell wrote:
    > > > Currently a parport_driver can't get a handle on the device node for the
    > > > underlying parport (PNPACPI, PCI, etc). That prevents correct placement
    > > > of sysfs child nodes, which can affect things like power management.
    > > >
    > > > This patch resolves that issue for non-legacy configurations:
    > > >
    > > > * "struct parport" now has a field pointing to that device node,
    > > > and non-legacy port drivers now initialize that device pointer:
    > > > - parport_mfc3 (can't test or build; no Amiga + Zorro here)
    > > > - parport_pc (and stop using only pci_device internally)
    > >
    > > Only in the PCI and PNP cases. Super-I/O and legacy cases still don't
    > > have a valid device pointer to pass. This annoys me because the laptop
    > > I'm using for my daily work has such a legacy parallel port,
    >
    > And SuperIO precludes PNP support? I guess that's one of the Mysteries.

    I don't think I have a Super-I/O chip in this laptop, and no PNP
    either (not for parport at least.) Most probably it has a totally
    legacy parallel port.

    > Well, as I said, "non-legacy" configs. One must start somewhere, and
    > I have no (working) legacy hardware to even try!

    Sure, the parport stuff is such a mess (showing its age I guess) that
    we can't hope to fix everything at once, and your patch is definitely a
    move in the right direction.

    > > Tested on my other machine with has a PNP parallel port too, and it
    > > worked fine. Great :)
    >
    > Good! Did you happen to try parport printing, with DMA?

    (/me looks at his brand new HP LaserJet P2015n network printer.)

    Parallel-port printers are soooo 20th century! ;)

    Seriously, no, the only parallel port devices I have are hardware
    monitoring evaluation boards. I can't test much advanced parport
    functionalities with these, I fear.

    > > > --- g26.orig/drivers/i2c/busses/i2c-parport.c 2006-12-12 19:25:43.000000000 -0800
    > > > +++ g26/drivers/i2c/busses/i2c-parport.c 2007-02-18 09:13:34.000000000 -0800
    > > > @@ -143,7 +143,7 @@ static struct i2c_algo_bit_data parport_
    > > >
    > > > /* ----- I2c and parallel port call-back functions and structures --------- */
    > > >
    > > > -static struct i2c_adapter parport_adapter = {
    > > > +static const struct i2c_adapter parport_adapter = {
    > > > .owner = THIS_MODULE,
    > > > .class = I2C_CLASS_HWMON,
    > > > .id = I2C_HW_B_LP,
    > >
    > > This change doesn't belong to this patch at all, although it is
    > > correct. I'll be happy to apply it if you send it to me separately.
    > >
    > > (Or it might be even better to get rid of this static structure
    > > altogether and intialize the fields of the dynamically allocated
    > > structure individually instead. This would make the driver smaller.)
    >
    > Smaller is better. ;)

    Patch written, I'll post it on the i2c list in a moment.

    > > Which means that I will have to fix the legacy parport_pc case right
    > > now, otherwise I will no longer be able to use i2c-parport and this
    > > isn't an option for me.
    >
    > I admire your enthusiasm. :)
    >
    >
    > > What do you think would be the right way to do
    > > it? A platform driver I guess, and we create a platform device for
    > > every successful call to parport_pc_probe_port() with a NULL dev
    > > pointer? That would be a fake driver, as the probe() and remove()
    > > methods would do nothing as far as I can see, but that's all I can
    > > think of at the moment.
    >
    > Yes, a platform_driver ... and ideally, moving all that hardware probing
    > and scanning code into a separate file. Probe/scan steps shouldn't really
    > be part of *any* driver.

    I fear I don't have the spare cycles to fulfill the "ideally" part.
    Here is the naive patch I have come up with. It does the job, even
    though it is not clean by any means. But as you said, it's certainly not
    worse than the current state, so I hope we can still apply it.

    * * * * *

    Give legacy parallel ports a platform device in the device tree.
    This is a quick and dirty implementation, it doesn't actually convert
    the legacy parport code to the device driver model, but at least
    parallel port device drivers will have a device to work with.

    Signed-off-by: Jean Delvare <khali@linux-fr.org>
    ---
    drivers/parport/parport_pc.c | 39 +++++++++++++++++++++++++++++++++++++++
    1 file changed, 39 insertions(+)
    --- linux-2.6.21-pre.orig/drivers/parport/parport_pc.c 2007-02-19 12:03:44.000000000 +0100
    +++ linux-2.6.21-pre/drivers/parport/parport_pc.c 2007-02-19 18:15:41.000000000 +0100
    @@ -53,6 +53,7 @@
    #include <linux/slab.h>
    #include <linux/pci.h>
    #include <linux/pnp.h>
    +#include <linux/platform_device.h>
    #include <linux/sysctl.h>

    #include <asm/io.h>
    @@ -2156,6 +2157,17 @@ struct parport *parport_pc_probe_port (u
    struct resource *base_res;
    struct resource *ECR_res = NULL;
    struct resource *EPP_res = NULL;
    + struct platform_device *pdev = NULL;
    +
    + if (!dev) {
    + /* We need a physical device to attach to, but none was
    + provided. Create our own. */
    + pdev = platform_device_register_simple("parport_pc",
    + base, NULL, 0);
    + if (IS_ERR(pdev))
    + return NULL;
    + dev = &pdev->dev;
    + }

    ops = kmalloc(sizeof (struct parport_operations), GFP_KERNEL);
    if (!ops)
    @@ -2359,6 +2371,8 @@ out3:
    out2:
    kfree (ops);
    out1:
    + if (pdev)
    + platform_device_unregister(pdev);
    return NULL;
    }

    @@ -3106,6 +3120,21 @@ static struct pnp_driver parport_pc_pnp_
    };


    +static int __devinit parport_pc_platform_probe(struct platform_device *pdev)
    +{
    + /* Always succeed, the actual probing is done in
    + parport_pc_probe_port(). */
    + return 0;
    +}
    +
    +static struct platform_driver parport_pc_platform_driver = {
    + .driver = {
    + .owner = THIS_MODULE,
    + .name = "parport_pc",
    + },
    + .probe = parport_pc_platform_probe,
    +};
    +
    /* This is called by parport_pc_find_nonpci_ports (in asm/parport.h) */
    static int __devinit __attribute__((unused))
    parport_pc_find_isa_ports (int autoirq, int autodma)
    @@ -3381,9 +3410,15 @@ __setup("parport_init_mode=",parport_ini

    static int __init parport_pc_init(void)
    {
    + int err;
    +
    if (parse_parport_params())
    return -EINVAL;

    + err = platform_driver_register(&parport_pc_platform_driver);
    + if (err)
    + return err;
    +
    if (io[0]) {
    int i;
    /* Only probe the ports we were given. */
    @@ -3408,6 +3443,7 @@ static void __exit parport_pc_exit(void)
    pci_unregister_driver (&parport_pc_pci_driver);
    if (pnp_registered_parport)
    pnp_unregister_driver (&parport_pc_pnp_driver);
    + platform_driver_unregister(&parport_pc_platform_driver);

    spin_lock(&ports_lock);
    while (!list_empty(&ports_list)) {
    @@ -3416,6 +3452,9 @@ static void __exit parport_pc_exit(void)
    priv = list_entry(ports_list.next,
    struct parport_pc_private, list);
    port = priv->port;
    + if (port->dev && port->dev->bus == &platform_bus_type)
    + platform_device_unregister(
    + to_platform_device(port->dev));
    spin_unlock(&ports_lock);
    parport_pc_unregister_port(port);
    spin_lock(&ports_lock);
    * * * * *

    > There are probably good reasons (== deep hardware braindamage on older
    > systems that are now hard to find) for the strange init sequencing in
    > that code, but I can't see why they should prevent splitting out
    >
    > (a) device discovery via probing, PNP, or whatever; from
    >
    > (b) the driver itself, getting handed a device node that's
    > pre-configured with the resources reported by discovery.
    >
    > Maybe the maintainers of the parport stack will have comments. Though
    > the info in MAINTAINERS seems dated, if not obsolete.

    Phil Blundell and Tim Waugh did not reply to me last time I sent a
    parport cleanup patch to them. I suspect they are indeed no longer
    maintaining parport in practice.

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

    \
     
     \ /
      Last update: 2007-02-20 22:13    [W:0.038 / U:58.904 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site