linux-kernel - Re: [PATCH 16/16] drivers/fsi: Add GPIO based FSI master

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] [day] [month] [year] [list]

Date:   Mon, 12 Dec 2016 13:49:41 -0600
From:   Christopher Bostic <christopher.lee.bostic@...il.com>
To:     Jeremy Kerr <jk@...abs.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>, linux@...linux.org.uk,
        Greg KH <gregkh@...uxfoundation.org>, sre@...nel.org,
        Michael Turquette <mturquette@...libre.com>,
        geert+renesas@...der.be,
        Open List OF Flattened dev tree bindings 
        <devicetree@...r.kernel.org>,
        "Moderated list: ARM PORT" <linux-arm-kernel@...ts.infradead.org>,
        Chris Bostic <cbostic@...ibm.com>,
        Joel Stanley <joel@....id.au>,
        Linux open list <linux-kernel@...r.kernel.org>,
        Andrew Jeffery <andrew@...id.au>,
        Alistair Popple <alistair@...ple.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH 16/16] drivers/fsi: Add GPIO based FSI master

On Thu, Dec 8, 2016 at 10:12 PM, Jeremy Kerr <jk@...abs.org> wrote:
> Hi Chris,
>
>> +static ssize_t store_scan(struct device *dev,
>> +                             struct device_attribute *attr,
>> +                             const char *buf,
>> +                             size_t count)
>> +{
>> +     struct fsi_master_gpio *master = dev_get_drvdata(dev);
>> +
>> +     fsi_master_gpio_init(master);
>> +
>> +     /* clear out any old scan data if present */
>> +     fsi_master_unregister(&master->master);
>> +     fsi_master_register(&master->master);
>> +
>> +     return count;
>> +}
>> +
>> +static DEVICE_ATTR(scan, 0200, NULL, store_scan);
>
> I think it would make more sense to have the scan attribute populated by
> the fsi core; we want this on all masters, not just GPIO.
>

Hi Jeremy,

Sure, will move that to the core.

> Currently, the only GPIO-master-specific functionality here is the
> fsi_master_gpio_init() - but isn't this something that we can do at
> probe time instead?
>

Yes that can be done at probe time.  Will change.

>> +static int fsi_master_gpio_probe(struct platform_device *pdev)
>> +{
>> +     struct fsi_master_gpio *master;
>> +     struct gpio_desc *gpio;
>> +
>> +     master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
>> +     if (!master)
>> +             return -ENOMEM;
>
> We should be populating master->dev.parent, see
>
>   https://github.com/jk-ozlabs/linux/commit/5225d6c47
>
>

Will make the change.

>> +     /* Optional pins */
>> +
>> +     gpio = devm_gpiod_get(&pdev->dev, "trans", 0);
>> +     if (IS_ERR(gpio))
>> +             dev_dbg(&pdev->dev, "probe: failed to get trans pin\n");
>> +     else
>> +             master->gpio_trans = gpio;
>
> I found devm_gpiod_get_optional(), which might make this a little
> neater.

Will make this change.


Thanks,
Chris

>
> Cheers,
>
>
> Jeremy