[lkml]   [2005]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] Eagle and ADI 930 usb adsl modem driver
    Hi Greg,

    thanks for your review.

    Greg KH wrote:
    > On Sun, Oct 30, 2005 at 12:37:41AM +0200, matthieu castet wrote:
    >>Please comment and consider for inclusion.
    > I need a "Signed-off-by:" line in order to be able to add it. Care to
    > redo things based on the comments you have had and resend it with this
    > line?
    no problem ;)
    >>+ *
    >>+ * This software is available to you under a choice of one of two
    >>+ * licenses. You may choose to be licensed under the terms of the GNU
    >>+ * General Public License (GPL) Version 2, available from the file
    >>+ * COPYING in the main directory of this source tree, or the
    >>+ * BSD license below:
    >>+ *
    >>+ * Redistribution and use in source and binary forms, with or without
    >>+ * modification, are permitted provided that the following conditions
    >>+ * are met:
    > <snip> You don't need the whole GPL 2 copy here, just put the first
    > paragraph you have before this one in.
    The paragraph you quote is the BSD licence, and point 1 is :
    Redistributions of source code must retain the above copyright
    * notice unmodified, this list of conditions, and the following
    * disclaimer

    So could I remove it ?

    >>+ * sometime hotplug don't have time to give the firmware the
    >>+ * first time, retry it.
    >>+ */
    >>+static int sleepy_request_firmware(const struct firmware **fw,
    >>+ const char *name, struct device *dev)
    >>+ if (request_firmware(fw, name, dev) == 0)
    >>+ return 0;
    >>+ msleep(1000);
    >>+ return request_firmware(fw, name, dev);
    > No, use the async firmware download mode instead of this. That will
    > solve all of your problems.
    Thanks, but does userspace will retry if it fails the first time ?
    The device needs the firmware quickly and after 3-5 seconds without it,
    it goes berserk.

    >>+/* we need to use semaphore until sysfs and removable devices is fixed
    >>+ * the problem is explained on
    >>+ */
    > This is the proper fix, why do you think it should be fixed in the
    > driver core?
    I don't remember, but aren't any possible race in sysfs code ?
    In the read, after the up, the usb_disconnect is scheduled, and call
    device_remove_file. Is that ok for the sysfs code to be still in the
    read code ?
    An other case : a process open a file, and start a read. Before the down
    , the usb_disconnect is scheduled and the module is removed.
    What will do the read, run code from the removed code ?

    >>diff -rNu -x '*.ko*' -x '*.mod*' -x '*.o*' linux-2.6.14/drivers/usb/atm.old/ueagle-atm.h linux-2.6.14/drivers/usb/atm/ueagle-atm.h
    >>--- linux-2.6.14/drivers/usb/atm.old/ueagle-atm.h 1970-01-01 01:00:00.000000000 +0100
    >>+++ linux-2.6.14/drivers/usb/atm/ueagle-atm.h 2005-10-30 00:25:27.000000000 +0200
    > Why do you need a header file for a single .c file?
    I think it makes things cleaner. I even like the bsd style where there
    is an header for reg (hardware values) and an other for val (driver

    >>+#define PACKED __attribute__ ((packed))
    > No, spell it out please.
    >>+/* structure describing a block within a DSP page */
    >>+typedef struct {
    >>+ __le16 wHdr;
    >>+#define UEA_BIHDR 0xabcd
    >>+ __le16 wAddress;
    >>+ __le16 wSize;
    >>+ __le16 wOvlOffset;
    >>+ __le16 wOvl; /* overlay */
    >>+ __le16 wLast;
    >>+} PACKED block_info_t;
    > Do not create new typedefs. Please get rid of all of them.
    It comes from bsd driver, it will be cleaned.



    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-11-02 21:18    [W:0.029 / U:6.028 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site