lkml.org 
[lkml]   [2019]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2 08/14] soundwire: intel: remove platform devices and provide new interface
    Date
    Use sdw_master_device and driver instead of platform devices

    To quote GregKH:

    "Don't mess with a platform device unless you really have no other
    possible choice. And even then, don't do it and try to do something
    else. Platform devices are really abused, don't perpetuate it "

    In addition, rather than a plain-vanilla init/exit, this patch
    provides 3 steps in the initialization (ACPI scan, probe, startup)
    which make it easier to verify support and allocate required resources
    as early as possible, and conversely help make the startup
    lighter-weight with only hardware register setup.

    Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
    ---
    drivers/soundwire/intel.c | 92 ++++++-----
    drivers/soundwire/intel.h | 8 +-
    drivers/soundwire/intel_init.c | 276 ++++++++++++++++++++++++---------
    3 files changed, 268 insertions(+), 108 deletions(-)

    diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
    index 64f97bb1a135..ba3bc410d816 100644
    --- a/drivers/soundwire/intel.c
    +++ b/drivers/soundwire/intel.c
    @@ -92,8 +92,6 @@
    #define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0)
    #define SDW_ALH_STRMZCFG_CHN GENMASK(19, 16)

    -#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
    -
    enum intel_pdi_type {
    INTEL_PDI_IN = 0,
    INTEL_PDI_OUT = 1,
    @@ -923,24 +921,23 @@ static int intel_init(struct sdw_intel *sdw)
    /*
    * probe and init
    */
    -static int intel_probe(struct platform_device *pdev)
    +static int intel_master_probe(struct sdw_master_device *md, void *link_ctx)
    {
    - struct sdw_cdns_stream_config config;
    struct sdw_intel *sdw;
    int ret;

    - sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL);
    + sdw = devm_kzalloc(&md->dev, sizeof(*sdw), GFP_KERNEL);
    if (!sdw)
    return -ENOMEM;

    - sdw->instance = pdev->id;
    - sdw->link_res = dev_get_platdata(&pdev->dev);
    - sdw->cdns.dev = &pdev->dev;
    + sdw->instance = md->link_id;
    + sdw->link_res = link_ctx;
    + sdw->cdns.dev = &md->dev;
    sdw->cdns.registers = sdw->link_res->registers;
    - sdw->cdns.instance = sdw->instance;
    + sdw->cdns.instance = md->link_id;
    sdw->cdns.msg_count = 0;
    - sdw->cdns.bus.dev = &pdev->dev;
    - sdw->cdns.bus.link_id = pdev->id;
    + sdw->cdns.bus.dev = &md->dev;
    + sdw->cdns.bus.link_id = md->link_id;

    sdw_cdns_probe(&sdw->cdns);

    @@ -948,16 +945,50 @@ static int intel_probe(struct platform_device *pdev)
    sdw_intel_ops.read_prop = intel_prop_read;
    sdw->cdns.bus.ops = &sdw_intel_ops;

    - platform_set_drvdata(pdev, sdw);
    + md->pdata = sdw;
    +
    + /* set driver data, accessed by snd_soc_dai_set_drvdata() */
    + dev_set_drvdata(&md->dev, &sdw->cdns);

    ret = sdw_add_bus_master(&sdw->cdns.bus);
    if (ret) {
    - dev_err(&pdev->dev, "sdw_add_bus_master fail: %d\n", ret);
    + dev_err(&md->dev, "sdw_add_bus_master fail: %d\n", ret);
    return ret;
    }

    if (sdw->cdns.bus.prop.hw_disabled) {
    - dev_info(&pdev->dev, "SoundWire master %d is disabled, ignoring\n",
    + dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n",
    + sdw->cdns.bus.link_id);
    + return 0;
    + }
    +
    + /* Acquire IRQ */
    + ret = request_threaded_irq(sdw->link_res->irq,
    + sdw_cdns_irq, sdw_cdns_thread,
    + IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
    + if (ret < 0) {
    + dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n",
    + sdw->link_res->irq);
    + goto err_init;
    + }
    +
    + return 0;
    +
    +err_init:
    + sdw_delete_bus_master(&sdw->cdns.bus);
    + return ret;
    +}
    +
    +static int intel_master_startup(struct sdw_master_device *md)
    +{
    + struct sdw_cdns_stream_config config;
    + struct sdw_intel *sdw;
    + int ret;
    +
    + sdw = md->pdata;
    +
    + if (sdw->cdns.bus.prop.hw_disabled) {
    + dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n",
    sdw->cdns.bus.link_id);
    return 0;
    }
    @@ -975,16 +1006,6 @@ static int intel_probe(struct platform_device *pdev)

    intel_pdi_ch_update(sdw);

    - /* Acquire IRQ */
    - ret = request_threaded_irq(sdw->link_res->irq,
    - sdw_cdns_irq, sdw_cdns_thread,
    - IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
    - if (ret < 0) {
    - dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n",
    - sdw->link_res->irq);
    - goto err_init;
    - }
    -
    ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
    if (ret < 0) {
    dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
    @@ -1011,17 +1032,17 @@ static int intel_probe(struct platform_device *pdev)

    err_interrupt:
    sdw_cdns_enable_interrupt(&sdw->cdns, false);
    - free_irq(sdw->link_res->irq, sdw);
    err_init:
    + free_irq(sdw->link_res->irq, sdw);
    sdw_delete_bus_master(&sdw->cdns.bus);
    return ret;
    }

    -static int intel_remove(struct platform_device *pdev)
    +static int intel_master_remove(struct sdw_master_device *md)
    {
    struct sdw_intel *sdw;

    - sdw = platform_get_drvdata(pdev);
    + sdw = md->pdata;

    if (!sdw->cdns.bus.prop.hw_disabled) {
    intel_debugfs_exit(sdw);
    @@ -1031,19 +1052,18 @@ static int intel_remove(struct platform_device *pdev)
    }
    sdw_delete_bus_master(&sdw->cdns.bus);

    + md->dev.driver = NULL;
    + device_unregister(&md->dev);
    +
    return 0;
    }

    -static struct platform_driver sdw_intel_drv = {
    - .probe = intel_probe,
    - .remove = intel_remove,
    - .driver = {
    - .name = "int-sdw",
    -
    - },
    +struct sdw_md_driver intel_sdw_driver = {
    + .probe = intel_master_probe,
    + .startup = intel_master_startup,
    + .remove = intel_master_remove,
    };
    -
    -module_platform_driver(sdw_intel_drv);
    +EXPORT_SYMBOL(intel_sdw_driver);

    MODULE_LICENSE("Dual BSD/GPL");
    MODULE_ALIAS("platform:int-sdw");
    diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
    index 38b7c125fb10..cfab2f00214d 100644
    --- a/drivers/soundwire/intel.h
    +++ b/drivers/soundwire/intel.h
    @@ -7,7 +7,7 @@
    /**
    * struct sdw_intel_link_res - Soundwire Intel link resource structure,
    * typically populated by the controller driver.
    - * @pdev: platform_device
    + * @md: master device
    * @mmio_base: mmio base of SoundWire registers
    * @registers: Link IO registers base
    * @shim: Audio shim pointer
    @@ -17,7 +17,7 @@
    * @dev: device implementing hw_params and free callbacks
    */
    struct sdw_intel_link_res {
    - struct platform_device *pdev;
    + struct sdw_master_device *md;
    void __iomem *mmio_base; /* not strictly needed, useful for debug */
    void __iomem *registers;
    void __iomem *shim;
    @@ -27,4 +27,8 @@ struct sdw_intel_link_res {
    struct device *dev;
    };

    +#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
    +
    +extern struct sdw_md_driver intel_sdw_driver;
    +
    #endif /* __SDW_INTEL_LOCAL_H */
    diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
    index 6bc167c83b47..42f7ae034bea 100644
    --- a/drivers/soundwire/intel_init.c
    +++ b/drivers/soundwire/intel_init.c
    @@ -11,7 +11,7 @@
    #include <linux/export.h>
    #include <linux/io.h>
    #include <linux/module.h>
    -#include <linux/platform_device.h>
    +#include <linux/soundwire/sdw.h>
    #include <linux/soundwire/sdw_intel.h>
    #include "intel.h"

    @@ -23,22 +23,47 @@
    #define SDW_LINK_BASE 0x30000
    #define SDW_LINK_SIZE 0x10000

    -static int link_mask;
    -module_param_named(sdw_link_mask, link_mask, int, 0444);
    +static int ctrl_link_mask;
    +module_param_named(sdw_link_mask, ctrl_link_mask, int, 0444);
    MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)");

    -static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
    +static bool is_link_enabled(struct fwnode_handle *fw_node, int i)
    +{
    + struct fwnode_handle *link;
    + char name[32];
    + u32 quirk_mask = 0;
    +
    + /* Find master handle */
    + snprintf(name, sizeof(name),
    + "mipi-sdw-link-%d-subproperties", i);
    +
    + link = fwnode_get_named_child_node(fw_node, name);
    + if (!link)
    + return false;
    +
    + fwnode_property_read_u32(link,
    + "intel-quirk-mask",
    + &quirk_mask);
    +
    + if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE)
    + return false;
    +
    + return true;
    +}
    +
    +static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx)
    {
    struct sdw_intel_link_res *link = ctx->links;
    + struct sdw_master_device *md;
    int i;

    if (!link)
    return 0;

    - for (i = 0; i < ctx->count; i++) {
    - if (link->pdev)
    - platform_device_unregister(link->pdev);
    - link++;
    + for (i = 0; i < ctx->count; i++, link++) {
    + md = link->md;
    + if (md)
    + md->driver->remove(md);
    }

    kfree(ctx->links);
    @@ -47,112 +72,194 @@ static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
    return 0;
    }

    -static struct sdw_intel_ctx
    -*sdw_intel_add_controller(struct sdw_intel_res *res)
    +static int
    +sdw_intel_scan_controller(struct sdw_intel_acpi_info *info)
    {
    - struct platform_device_info pdevinfo;
    - struct platform_device *pdev;
    - struct sdw_intel_link_res *link;
    - struct sdw_intel_ctx *ctx;
    struct acpi_device *adev;
    int ret, i;
    u8 count;
    - u32 caps;

    - if (acpi_bus_get_device(res->handle, &adev))
    - return NULL;
    + if (acpi_bus_get_device(info->handle, &adev))
    + return -EINVAL;

    /* Found controller, find links supported */
    count = 0;
    ret = fwnode_property_read_u8_array(acpi_fwnode_handle(adev),
    "mipi-sdw-master-count", &count, 1);

    - /* Don't fail on error, continue and use hw value */
    + /*
    + * In theory we could check the number of links supported in
    + * hardware, but in that step we cannot assume SoundWire IP is
    + * powered.
    + *
    + * In addition, if the BIOS doesn't even provide this
    + * 'master-count' property then all the inits based on link
    + * masks will fail as well.
    + *
    + * We will check the hardware capabilities in the startup() step
    + */
    +
    if (ret) {
    dev_err(&adev->dev,
    "Failed to read mipi-sdw-master-count: %d\n", ret);
    - count = SDW_MAX_LINKS;
    + return -EINVAL;
    }

    - /* Check SNDWLCAP.LCOUNT */
    - caps = ioread32(res->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP);
    - caps &= GENMASK(2, 0);
    -
    - /* Check HW supported vs property value and use min of two */
    - count = min_t(u8, caps, count);
    -
    /* Check count is within bounds */
    if (count > SDW_MAX_LINKS) {
    dev_err(&adev->dev, "Link count %d exceeds max %d\n",
    count, SDW_MAX_LINKS);
    - return NULL;
    + return -EINVAL;
    } else if (!count) {
    dev_warn(&adev->dev, "No SoundWire links detected\n");
    - return NULL;
    + return -EINVAL;
    }
    + dev_dbg(&adev->dev, "ACPI reports %d SDW Link devices\n", count);
    +
    + info->count = count;

    + for (i = 0; i < count; i++) {
    + if (ctrl_link_mask && !(ctrl_link_mask & BIT(i))) {
    + dev_dbg(&adev->dev,
    + "Link %d masked, will not be enabled\n", i);
    + continue;
    + }
    +
    + if (!is_link_enabled(acpi_fwnode_handle(adev), i)) {
    + dev_dbg(&adev->dev,
    + "Link %d not selected in firmware\n", i);
    + continue;
    + }
    +
    + info->link_mask |= BIT(i);
    + }
    +
    + return 0;
    +}
    +
    +static struct sdw_intel_ctx
    +*sdw_intel_probe_controller(struct sdw_intel_res *res)
    +{
    + struct sdw_intel_link_res *link;
    + struct sdw_intel_ctx *ctx;
    + struct acpi_device *adev;
    + struct sdw_master_device *md;
    + u32 link_mask;
    + int count;
    + int i;
    +
    + if (!res)
    + return NULL;
    +
    + if (acpi_bus_get_device(res->handle, &adev))
    + return NULL;
    +
    + if (!res->count)
    + return NULL;
    +
    + count = res->count;
    dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);

    ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
    if (!ctx)
    return NULL;

    - ctx->count = count;
    - ctx->links = kcalloc(ctx->count, sizeof(*ctx->links), GFP_KERNEL);
    + ctx->links = kcalloc(count, sizeof(*ctx->links), GFP_KERNEL);
    if (!ctx->links)
    goto link_err;

    + ctx->count = count;
    + ctx->mmio_base = res->mmio_base;
    + ctx->link_mask = res->link_mask;
    + ctx->handle = res->handle;
    +
    link = ctx->links;
    + link_mask = ctx->link_mask;

    /* Create SDW Master devices */
    - for (i = 0; i < count; i++) {
    - if (link_mask && !(link_mask & BIT(i))) {
    - dev_dbg(&adev->dev,
    - "Link %d masked, will not be enabled\n", i);
    - link++;
    + for (i = 0; i < count; i++, link++) {
    + if (link_mask && !(link_mask & BIT(i)))
    continue;
    - }

    + md = sdw_md_add(&intel_sdw_driver,
    + res->parent,
    + acpi_fwnode_handle(adev),
    + i);
    +
    + if (IS_ERR(md)) {
    + dev_err(&adev->dev, "Could not create link %d\n", i);
    + goto err;
    + }
    + link->md = md;
    + link->mmio_base = res->mmio_base;
    link->registers = res->mmio_base + SDW_LINK_BASE
    - + (SDW_LINK_SIZE * i);
    + + (SDW_LINK_SIZE * i);
    link->shim = res->mmio_base + SDW_SHIM_BASE;
    link->alh = res->mmio_base + SDW_ALH_BASE;
    -
    + link->irq = res->irq;
    link->ops = res->ops;
    link->dev = res->dev;

    - memset(&pdevinfo, 0, sizeof(pdevinfo));
    -
    - pdevinfo.parent = res->parent;
    - pdevinfo.name = "int-sdw";
    - pdevinfo.id = i;
    - pdevinfo.fwnode = acpi_fwnode_handle(adev);
    -
    - pdev = platform_device_register_full(&pdevinfo);
    - if (IS_ERR(pdev)) {
    - dev_err(&adev->dev,
    - "platform device creation failed: %ld\n",
    - PTR_ERR(pdev));
    - goto pdev_err;
    - }
    -
    - link->pdev = pdev;
    - link++;
    + /* let the SoundWire master driver to its probe */
    + md->driver->probe(md, link);
    }

    return ctx;

    -pdev_err:
    - sdw_intel_cleanup_pdev(ctx);
    +err:
    + sdw_intel_cleanup(ctx);
    link_err:
    kfree(ctx);
    return NULL;
    }

    +static int
    +sdw_intel_startup_controller(struct sdw_intel_ctx *ctx)
    +{
    + struct acpi_device *adev;
    + struct sdw_intel_link_res *link;
    + struct sdw_master_device *md;
    + u32 caps;
    + u32 link_mask;
    + int i;
    +
    + if (acpi_bus_get_device(ctx->handle, &adev))
    + return -EINVAL;
    +
    + /* Check SNDWLCAP.LCOUNT */
    + caps = ioread32(ctx->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP);
    + caps &= GENMASK(2, 0);
    +
    + /* Check HW supported vs property value */
    + if (caps < ctx->count) {
    + dev_err(&adev->dev,
    + "BIOS master count is larger than hardware capabilities\n");
    + return -EINVAL;
    + }
    +
    + if (!ctx->links)
    + return -EINVAL;
    +
    + link = ctx->links;
    + link_mask = ctx->link_mask;
    +
    + /* Create SDW Master devices */
    + for (i = 0; i < ctx->count; i++, link++) {
    + if (link_mask && !(link_mask & BIT(i)))
    + continue;
    +
    + md = link->md;
    +
    + md->driver->startup(md);
    + }
    +
    + return 0;
    +}
    +
    static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
    void *cdata, void **return_value)
    {
    - struct sdw_intel_res *res = cdata;
    + struct sdw_intel_acpi_info *info = cdata;
    struct acpi_device *adev;
    acpi_status status;
    u64 adr;
    @@ -166,7 +273,7 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
    return AE_NOT_FOUND;
    }

    - res->handle = handle;
    + info->handle = handle;

    /*
    * On some Intel platforms, multiple children of the HDAS
    @@ -183,37 +290,66 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
    }

    /**
    - * sdw_intel_init() - SoundWire Intel init routine
    + * sdw_intel_acpi_scan() - SoundWire Intel init routine
    * @parent_handle: ACPI parent handle
    - * @res: resource data
    + * @info: description of what firmware/DSDT tables expose
    *
    - * This scans the namespace and creates SoundWire link controller devices
    - * based on the info queried.
    + * This scans the namespace and queries firmware to figure out which
    + * links to enable. A follow-up use of sdw_intel_probe() and
    + * sdw_intel_startup() is required for creation of devices and bus
    + * startup
    */
    -void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res)
    +int sdw_intel_acpi_scan(acpi_handle *parent_handle,
    + struct sdw_intel_acpi_info *info)
    {
    acpi_status status;

    status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
    parent_handle, 1,
    sdw_intel_acpi_cb,
    - NULL, res, NULL);
    + NULL, info, NULL);
    if (ACPI_FAILURE(status))
    - return NULL;
    + return -ENODEV;

    - return sdw_intel_add_controller(res);
    + return sdw_intel_scan_controller(info);
    }
    -EXPORT_SYMBOL(sdw_intel_init);
    +EXPORT_SYMBOL(sdw_intel_acpi_scan);

    +/**
    + * sdw_intel_probe() - SoundWire Intel probe routine
    + * @parent_handle: ACPI parent handle
    + * @res: resource data
    + *
    + * This creates SoundWire Master and Slave devices below the controller.
    + * All the information necessary is stored in the context, and the res
    + * argument pointer can be freed after this step.
    + */
    +struct sdw_intel_ctx
    +*sdw_intel_probe(struct sdw_intel_res *res)
    +{
    + return sdw_intel_probe_controller(res);
    +}
    +EXPORT_SYMBOL(sdw_intel_probe);
    +
    +/**
    + * sdw_intel_startup() - SoundWire Intel startup
    + * @ctx: SoundWire context allocated in the probe
    + *
    + */
    +int sdw_intel_startup(struct sdw_intel_ctx *ctx)
    +{
    + return sdw_intel_startup_controller(ctx);
    +}
    +EXPORT_SYMBOL(sdw_intel_startup);
    /**
    * sdw_intel_exit() - SoundWire Intel exit
    - * @arg: callback context
    + * @ctx: SoundWire context allocated in the probe
    *
    * Delete the controller instances created and cleanup
    */
    void sdw_intel_exit(struct sdw_intel_ctx *ctx)
    {
    - sdw_intel_cleanup_pdev(ctx);
    + sdw_intel_cleanup(ctx);
    kfree(ctx);
    }
    EXPORT_SYMBOL(sdw_intel_exit);
    --
    2.20.1
    \
     
     \ /
      Last update: 2019-11-06 20:14    [W:4.768 / U:0.248 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site