Re: [PATCH v2] i2c-designware: Add suport for AMD i2c controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mika,

Excuse me, Could I consult one more question with you?
Could I add the AMD i2c controller private data like clk rate in the
platform_driver-->id_table but not in the
platform_driver-->acpi_match_table? The cause is as follow:


As you suggestion, I try to pass the clock rate in the ->driver_data,
so I add a line in the
dw_i2c_acpi_match[] array (drivers/i2c/busses/i2c-designware-platdrv.c):

       static const struct acpi_device_id dw_i2c_acpi_match[] = {
                      ...
                      { "AMD0010", (unsigned long)&amd_i2c_config}
                      ...
       }
       define the clock rate as a member of the amd_i2c_config structure.

But I find that I can not pass the clock rate by this way, since when
platform device match with with platform driver,
The driver_data of platform_driver-->acpi_match_table will not be
assigned to pdev->id_entry,
just the driver_data of platform_driver-->id_table will be assigned to
pdev->id_entry, as you know, AMD i2c controller
is a ACPI device, should use platform_driver-->acpi_match_table not
platform_driver-->id_table.

platform_match(drivers/base/platform.c)
|
acpi_driver_match_device(dev, drv)
|
return !!acpi_match_device(drv->acpi_match_table, dev);
|
return __acpi_match_device(adev, ids)(drivers/acpi/scan.c)
|
return id
Just return id, but will not assign id to pdev->id_entry.

Really expect your suggestion!
Thanks a lot!

Carl

On Fri, Sep 5, 2014 at 5:36 PM, Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> On Fri, Sep 05, 2014 at 03:19:50PM +0800, carl peng wrote:
>> Hi Mika,
>>
>> Could I consult a problem with you?
>
> Sure.
>
>>
>> As you suggest, there are some samples that add the device clk to the
>> clock framework in the acpi_lpss.c. But I encounter some problems when
>> add the AMD i2c clk to the clock framework.
>> In acpi_lpss.c, as the patch d6ddaaac8f5c37ad84d said, When the
>> CONFIG_X86_INTEL_LPSS is not defined, then only add the device ID
>> list, but not implement the callbacks like "attach =
>> acpi_lpss_create_device" , so if
>> CONFIG_X86_INTEL_LPSS is not defined, will not create clk dev.
>>
>> So there will be some problems when I add the AMD i2c clk to the clock
>> framework.
>> if I do not define CONFIG_X86_INTEL_LPSS, then even though I add the IDs and
>> lpss_device_desc struct into the acpi_lpss.c, the AMD i2c clk will
>> also can not  be added into the clock framework. But if I add the AMD
>> i2c related IDs and struct in the CONFIG_X86_INTEL_LPSS, it may cause
>> some misunderstand in the future, it is a AMD device, why is it put
>> into the CONFIG_X86_INTEL_LPSS branch.
>>
>> So, Could you please give some suggestions to me?
>> Which should I choose?
>> Add the AMD i2c clk IDs and related lpss_device_desc into the
>> CONFIG_X86_INTEL_LPSS branch, or re-write a new branch by imitating
>> the
>> CONFIG_X86_INTEL_LPSS branch  included by CONFIG_AMD_INTEL_LPSS.
>
> Since the AMD part is probably too different from Intel LPSS devices, I
> would rather not add it there. So you have two options AFAICT:
>
>  1) Add the clock in drivers/acpi/acpi_platform.c
>  2) Create custom i2c_get_clk_rate_khz() in the driver, just like in
>     this patch but drop all the unnecessary vendor etc. dance.
>
> For the 2) you could pass the clock rate in ->driver_data and if it is
> non-zero, setup a custom clock rate function that then uses the value
> from ->driver_data or so.
>
> Not sure if Wolfram likes that but we don't have too many places in x86
> world where we can create clocks :-(
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux