linux-kernel - Re: [PATCH anybus v4 1/7] fieldbus_dev: add Fieldbus Device subsystem.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGngYiUL7bZVEehj4LGC2H+kv+Z4v_s=tG4uosOp1-Rc4_pGWw@mail.gmail.com>
Date:   Wed, 28 Nov 2018 10:39:41 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     gregkh@...uxfoundation.org
Cc:     Sven Van Asbroeck <svendev@...x.com>, robh+dt@...nel.org,
        Linus Walleij <linus.walleij@...aro.org>,
        Lee Jones <lee.jones@...aro.org>, mark.rutland@....com,
        Andreas Färber <afaerber@...e.de>,
        treding@...dia.com, David Lechner <david@...hnology.com>,
        noralf@...nnes.org, johan@...nel.org,
        Michal Simek <monstr@...str.eu>, michal.vokac@...ft.com,
        Arnd Bergmann <arnd@...db.de>, john.garry@...wei.com,
        geert+renesas@...der.be, robin.murphy@....com,
        paul.gortmaker@...driver.com,
        sebastien.bourdelin@...oirfairelinux.com, icenowy@...c.io,
        Stuart Yoder <stuyoder@...il.com>, maxime.ripard@...tlin.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>
Subject: Re: [PATCH anybus v4 1/7] fieldbus_dev: add Fieldbus Device subsystem.

Wow Greg, thanks for the review, this is awesome !!

On Tue, Nov 27, 2018 at 2:54 AM Greg KH <gregkh@...uxfoundation.org> wrote:

>> +     cdev_init(&fb->cdev, &fieldbus_fops);
>> +     err = cdev_add(&fb->cdev, devno, 1);
>> +     if (err) {
>> +             pr_err("fieldbus_dev%d unable to add device %d:%d\n",
>> +                     fb->id, MAJOR(fieldbus_devt), fb->id);
>> +             goto err_cdev;
>> +     }
>
> Why do you have a static cdev?

The proposed fieldbus API needs a single /dev/fieldbus_devX node for every
device. I just looked around the drivers/ tree to see how others accomplish
this.

Is there a better way?

>> +     fb->online_sd = sysfs_get_dirent(fb->dev->kobj.sd, "online");
>
> Ick, what?  Why?  Why are you messing around with a raw sysfs attribute?

The proposed fieldbus API has a sysfs attribute that can be poll/select'ed on.
Is this behaviour still allowed / ok?
If so, you're saying that I should not store the raw attribute, but just do:
sysfs_notify(&fb->dev->kobj, NULL, "online") ?

Now that I (hopefully) have a few seconds of your attention...
I suppose the fieldbus API in this patch can't go anywhere, without buy-in from
multiple people who also want to use fieldbus. Right now, there are none.

This might be a chicken-and-egg problem. Perhaps here are no fieldbus
devices because
there's no good general API. There's no good general API because there are no
fieldbus devices yet.

 Is there a tried and tested way to break this deadlock?