lkml.org 
[lkml]   [2016]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver
    From
    Date
    Am Donnerstag, den 03.03.2016, 15:29 +0100 schrieb Neil Armstrong:
    > >> +static int oxnas_reset_reset(struct reset_controller_dev *rcdev,
    > >> + unsigned long id)
    > >> +{
    > >> + struct oxnas_reset *data =
    > >> + container_of(rcdev, struct oxnas_reset, rcdev);
    > >> +
    > >> + regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
    > >> + msleep(50);
    > >
    > > Is this the right delay for all of the resets in this register?
    > > If not, I'd drop the .reset callback.
    > >
    > The delay is not strictly necessary, but better to avoid any HW issues.

    Ok, maybe add a comment.

    > And the .reset callback is needed since reset_control_reset
    > does not assert -> deassert as fallback.

    That's because some controllers don't even have manual
    assertion/deassertion, and for some reset lines the drivers better know
    the timing or they want to do other stuff while the reset is asserted.

    [...]
    > >> +static struct reset_control_ops oxnas_reset_ops = {
    > >
    > > const
    > >
    > Something checkpatch should report...

    This is new in any case. rcdev->ops was not const* until recently.

    > >> + .reset = oxnas_reset_reset,
    > >> + .assert = oxnas_reset_assert,
    > >> + .deassert = oxnas_reset_deassert,
    > >> +};
    > >> +
    > >> +static const struct of_device_id oxnas_reset_dt_ids[] = {
    > >> + { .compatible = "plxtech,nas782x-reset", },
    > >> + { /* sentinel */ },
    > >> +};
    > >> +MODULE_DEVICE_TABLE(of, oxnas_reset_dt_ids);
    > >> +
    > >> +static int oxnas_reset_probe(struct platform_device *pdev)
    > >> +{
    > >> + struct oxnas_reset *data;
    > >> + struct device *parent;
    > >> +
    > >> + parent = pdev->dev.parent;
    > >> + if (!parent) {
    > >> + dev_err(&pdev->dev, "no parent\n");
    > >
    > > Can this even happen?
    > >
    > It's to make sure parent->of_node is valid for syscon_node_to_regmap.

    Since this is a platform device probed via device tree,
    pdev->dev.parent should always be set (see of_device_alloc()).

    regards
    Philipp

    \
     
     \ /
      Last update: 2016-03-03 16:41    [W:4.942 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site