From: Christopher Bostic <christopher.lee.bostic@gmail.com>
To: Jeremy Kerr <jk@ozlabs.org>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
linux@armlinux.org.uk, Greg KH <gregkh@linuxfoundation.org>,
sre@kernel.org, Michael Turquette <mturquette@baylibre.com>,
geert+renesas@glider.be,
Open List OF Flattened dev tree bindings
<devicetree@vger.kernel.org>,
"Moderated list: ARM PORT" <linux-arm-kernel@lists.infradead.org>,
Chris Bostic <cbostic@us.ibm.com>, Joel Stanley <joel@jms.id.au>,
Linux open list <linux-kernel@vger.kernel.org>,
Andrew Jeffery <andrew@aj.id.au>,
Alistair Popple <alistair@popple.id.au>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [PATCH 16/16] drivers/fsi: Add GPIO based FSI master
Date: Mon, 12 Dec 2016 13:49:41 -0600 [thread overview]
Message-ID: <CAA0LjjV98roN5MEyfkRFf8m61C_nHi6Wxq_2wHB=c6YQ35_wUQ@mail.gmail.com> (raw)
In-Reply-To: <6a39f4d9-0f20-a146-3122-86d3f75c58fa@ozlabs.org>
On Thu, Dec 8, 2016 at 10:12 PM, Jeremy Kerr <jk@ozlabs.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
next prev parent reply other threads:[~2016-12-12 19:49 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-07 0:14 [PATCH 00/16] FSI device driver introduction Chris Bostic
2016-12-07 0:14 ` [PATCH 01/16] drivers/fsi: Add empty fsi bus definitions Chris Bostic
2016-12-07 0:14 ` [PATCH 02/16] drivers/fsi: Add device & driver definitions Chris Bostic
2016-12-07 0:14 ` [PATCH 03/16] drivers/fsi: add driver to device matches Chris Bostic
2016-12-07 0:14 ` [PATCH 05/16] drivers/fsi: Add fake master driver Chris Bostic
2016-12-07 12:09 ` Mark Rutland
2016-12-07 23:27 ` Jeremy Kerr
2016-12-07 0:14 ` [PATCH 06/16] drivers/fsi: Add slave definition Chris Bostic
2016-12-07 0:14 ` [PATCH 07/16] drivers/fsi: Add empty master scan Chris Bostic
2016-12-07 0:14 ` [PATCH 10/16] drivers/fsi: scan slaves & register devices Chris Bostic
2016-12-07 0:14 ` [PATCH 13/16] drivers/fsi: Set slave SMODE to init communication Chris Bostic
2016-12-07 0:14 ` [PATCH 14/16] drivers/fsi: Add master unscan Chris Bostic
2016-12-07 9:31 ` Greg KH
2016-12-07 0:14 ` [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings Chris Bostic
2016-12-07 12:02 ` Mark Rutland
2016-12-07 0:14 ` [PATCH 16/16] drivers/fsi: Add GPIO based FSI master Chris Bostic
2016-12-09 4:12 ` Jeremy Kerr
2016-12-12 19:49 ` Christopher Bostic [this message]
2016-12-07 1:52 ` [PATCH 00/16] FSI device driver introduction Sebastian Reichel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAA0LjjV98roN5MEyfkRFf8m61C_nHi6Wxq_2wHB=c6YQ35_wUQ@mail.gmail.com' \
--to=christopher.lee.bostic@gmail.com \
--cc=alistair@popple.id.au \
--cc=andrew@aj.id.au \
--cc=benh@kernel.crashing.org \
--cc=cbostic@us.ibm.com \
--cc=devicetree@vger.kernel.org \
--cc=geert+renesas@glider.be \
--cc=gregkh@linuxfoundation.org \
--cc=jk@ozlabs.org \
--cc=joel@jms.id.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mark.rutland@arm.com \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sre@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).