On 7/16/19 1:26 PM, Chanwoo Choi wrote:
> Hi,
>
> On 19. 7. 16. 오후 7:59, Bartlomiej Zolnierkiewicz wrote:
>>
>> On 7/16/19 12:33 PM, Chanwoo Choi wrote:
>>> Hi Bartlomiej,
>>>
>>> On 19. 7. 16. 오후 7:13, Bartlomiej Zolnierkiewicz wrote:
>>>>
>>>> Hi Chanwoo,
>>>>
>>>> On 7/16/19 5:56 AM, Chanwoo Choi wrote:
>>>>> Hi Kamil,
>>>>>
>>>>> Looks good to me. But, this patch has some issue.
>>>>> I added the detailed reviews.
>>>>>
>>>>> I recommend that you make the separate patches as following
>>>>> in order to clarify the role of which apply the dev_pm_opp_* function.
>>>>>
>>>>> First patch,
>>>>> Need to consolidate the following two function into one function.
>>>>> because the original exynos-bus.c has the problem that the regulator
>>>>> of parent devfreq device have to be enabled before enabling the clock.
>>>>> This issue did not happen because bootloader enables the bus-related
>>>>> regulators before kernel booting.
>>>>> - exynos_bus_parse_of()
>>>>> - exynos_bus_parent_parse_of()
>>>>>> Second patch,
>>>>> Apply dev_pm_opp_set_regulators() and dev_pm_opp_set_rate()
>>>>>
>>>>>
>>>>> On 19. 7. 15. 오후 9:04, Kamil Konieczny wrote:
>>>>>> Reuse opp core code for setting bus clock and voltage. As a side
>>>>>> effect this allow useage of coupled regulators feature (required
>>>>>> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
>>>>>> uses regulator_set_voltage_triplet() for setting regulator voltage
>>>>>> while the old code used regulator_set_voltage_tol() with fixed
>>>>>> tolerance. This patch also removes no longer needed parsing of DT
>>>>>> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
>>>>>> it).
>>>>>>
>>>>>> Signed-off-by: Kamil Konieczny <k.konieczny@xxxxxxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>> drivers/devfreq/exynos-bus.c | 172 ++++++++++++++---------------------
>>>>>> 1 file changed, 66 insertions(+), 106 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>>>>> index 486cc5b422f1..7fc4f76bd848 100644
>>>>>> --- a/drivers/devfreq/exynos-bus.c
>>>>>> +++ b/drivers/devfreq/exynos-bus.c
>>>>>> @@ -25,7 +25,6 @@
>>>>>> #include <linux/slab.h>
>>>>>>
>>>>>> #define DEFAULT_SATURATION_RATIO 40
>>>>>> -#define DEFAULT_VOLTAGE_TOLERANCE 2
>>>>>>
>>>>>> struct exynos_bus {
>>>>>> struct device *dev;
>>>>>> @@ -37,9 +36,9 @@ struct exynos_bus {
>>>>>>
>>>>>> unsigned long curr_freq;
>>>>>>
>>>>>> - struct regulator *regulator;
>>>>>> + struct opp_table *opp_table;
>>>>>> +
>>>>>> struct clk *clk;
>>>>>> - unsigned int voltage_tolerance;
>>>>>> unsigned int ratio;
>>>>>> };
>>>>>>
>>>>>> @@ -99,56 +98,25 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
>>>>>> {
>>>>>> struct exynos_bus *bus = dev_get_drvdata(dev);
>>>>>> struct dev_pm_opp *new_opp;
>>>>>> - unsigned long old_freq, new_freq, new_volt, tol;
>>>>>> int ret = 0;
>>>>>> -
>>>>>> - /* Get new opp-bus instance according to new bus clock */
>>>>>> + /*
>>>>>> + * New frequency for bus may not be exactly matched to opp, adjust
>>>>>> + * *freq to correct value.
>>>>>> + */
>>>>>
>>>>> You better to change this comment with following styles
>>>>> to keep the consistency:
>>>>>
>>>>> /* Get correct frequency for bus ... */
>>>>>
>>>>>> new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>>> if (IS_ERR(new_opp)) {
>>>>>> dev_err(dev, "failed to get recommended opp instance\n");
>>>>>> return PTR_ERR(new_opp);
>>>>>> }
>>>>>>
>>>>>> - new_freq = dev_pm_opp_get_freq(new_opp);
>>>>>> - new_volt = dev_pm_opp_get_voltage(new_opp);
>>>>>> dev_pm_opp_put(new_opp);
>>>>>>
>>>>>> - old_freq = bus->curr_freq;
>>>>>> -
>>>>>> - if (old_freq == new_freq)
>>>>>> - return 0;
>>>>>> - tol = new_volt * bus->voltage_tolerance / 100;
>>>>>> -
>>>>>> /* Change voltage and frequency according to new OPP level */
>>>>>> mutex_lock(&bus->lock);
>>>>>> + ret = dev_pm_opp_set_rate(dev, *freq);
>>>>>> + if (!ret)
>>>>>> + bus->curr_freq = *freq;
>>>>>
>>>>> Have to print the error log if ret has minus error value.
>>>>
>>>> dev_pm_opp_set_rate() should print the error message on all
>>>> errors so wouldn't printing the error log also here be superfluous?
>>>>
>>>> [ Please also note that the other user of dev_pm_opp_set_rate()
>>>> (cpufreq-dt cpufreq driver) doesn't do this. ]
>>>
>>> OK. Thanks for the explanation.
>>>
>>>>
>>>>> Modify it as following:
>>>>>
>>>>> if (ret < 0) {
>>>>> dev_err(dev, "failed to set bus rate\n");
>>>>> goto err:
>>>>> }
>>>>> bus->curr_freq = *freq;
>>>>>
>>>>> err:
>>>>> mutex_unlock(&bus->lock);
>>>>>
>>>>> return ret;
>>>>>
>>>>>>
>>>>>> - if (old_freq < new_freq) {
>>>>>> - ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>>>>>> - if (ret < 0) {
>>>>>> - dev_err(bus->dev, "failed to set voltage\n");
>>>>>> - goto out;
>>>>>> - }
>>>>>> - }
>>>>>> -
>>>>>> - ret = clk_set_rate(bus->clk, new_freq);
>>>>>> - if (ret < 0) {
>>>>>> - dev_err(dev, "failed to change clock of bus\n");
>>>>>> - clk_set_rate(bus->clk, old_freq);
>>>>>> - goto out;
>>>>>> - }
>>>>>> -
>>>>>> - if (old_freq > new_freq) {
>>>>>> - ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>>>>>> - if (ret < 0) {
>>>>>> - dev_err(bus->dev, "failed to set voltage\n");
>>>>>> - goto out;
>>>>>> - }
>>>>>> - }
>>>>>> - bus->curr_freq = new_freq;
>>>>>> -
>>>>>> - dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>>>>>> - old_freq, new_freq, clk_get_rate(bus->clk));
>>>>>> -out:
>>>>>> mutex_unlock(&bus->lock);
>>>>>>
>>>>>> return ret;
>>>>>> @@ -194,10 +162,11 @@ static void exynos_bus_exit(struct device *dev)
>>>>>> if (ret < 0)
>>>>>> dev_warn(dev, "failed to disable the devfreq-event devices\n");
>>>>>>
>>>>>> - if (bus->regulator)
>>>>>> - regulator_disable(bus->regulator);
>>>>>> + if (bus->opp_table)
>>>>>> + dev_pm_opp_put_regulators(bus->opp_table);
>>>>>
>>>>> Have to disable regulator after disabling the clock
>>>>> to prevent the h/w fault.
>>>>>
>>>>> I think that you should call them with following sequence:
>>>>>
>>>>> clk_disable_unprepare(bus->clk);
>>>>> if (bus->opp_table)
>>>>> dev_pm_opp_put_regulators(bus->opp_table);
>>>>> dev_pm_opp_of_remove_table(dev);
>>>>>
>>>>>>
>>>>>> dev_pm_opp_of_remove_table(dev);
>>>>>> +
>>>>>> clk_disable_unprepare(bus->clk);
>>>>>> }
>>>>>>
>>>>>> @@ -209,39 +178,26 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
>>>>>> {
>>>>>> struct exynos_bus *bus = dev_get_drvdata(dev);
>>>>>> struct dev_pm_opp *new_opp;
>>>>>> - unsigned long old_freq, new_freq;
>>>>>> - int ret = 0;
>>>>>> + int ret;
>>>>>>
>>>>>> - /* Get new opp-bus instance according to new bus clock */
>>>>>> + /*
>>>>>> + * New frequency for bus may not be exactly matched to opp, adjust
>>>>>> + * *freq to correct value.
>>>>>> + */
>>>>>
>>>>> You better to change this comment with following styles
>>>>> to keep the consistency:
>>>>>
>>>>> /* Get correct frequency for bus ... */
>>>>>
>>>>>> new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>>> if (IS_ERR(new_opp)) {
>>>>>> dev_err(dev, "failed to get recommended opp instance\n");
>>>>>> return PTR_ERR(new_opp);
>>>>>> }
>>>>>>
>>>>>> - new_freq = dev_pm_opp_get_freq(new_opp);
>>>>>> dev_pm_opp_put(new_opp);
>>>>>>
>>>>>> - old_freq = bus->curr_freq;
>>>>>> -
>>>>>> - if (old_freq == new_freq)
>>>>>> - return 0;
>>>>>> -
>>>>>> /* Change the frequency according to new OPP level */
>>>>>> mutex_lock(&bus->lock);
>>>>>> + ret = dev_pm_opp_set_rate(dev, *freq);
>>>>>> + if (!ret)
>>>>>> + bus->curr_freq = *freq;
>>>>>
>>>>> ditto. Have to print the error log, check above comment.
>>>>>
>>>>>>
>>>>>> - ret = clk_set_rate(bus->clk, new_freq);
>>>>>> - if (ret < 0) {
>>>>>> - dev_err(dev, "failed to set the clock of bus\n");
>>>>>> - goto out;
>>>>>> - }
>>>>>> -
>>>>>> - *freq = new_freq;
>>>>>> - bus->curr_freq = new_freq;
>>>>>> -
>>>>>> - dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>>>>>> - old_freq, new_freq, clk_get_rate(bus->clk));
>>>>>> -out:
>>>>>> mutex_unlock(&bus->lock);
>>>>>>
>>>>>> return ret;
>>>>>> @@ -259,20 +215,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>>> struct exynos_bus *bus)
>>>>>> {
>>>>>> struct device *dev = bus->dev;
>>>>>> - int i, ret, count, size;
>>>>>> -
>>>>>> - /* Get the regulator to provide each bus with the power */
>>>>>> - bus->regulator = devm_regulator_get(dev, "vdd");
>>>>>> - if (IS_ERR(bus->regulator)) {
>>>>>> - dev_err(dev, "failed to get VDD regulator\n");
>>>>>> - return PTR_ERR(bus->regulator);
>>>>>> - }
>>>>>> -
>>>>>> - ret = regulator_enable(bus->regulator);
>>>>>> - if (ret < 0) {
>>>>>> - dev_err(dev, "failed to enable VDD regulator\n");
>>>>>> - return ret;
>>>>>> - }
>>>>>> + int i, count, size;
>>>>>>
>>>>>> /*
>>>>>> * Get the devfreq-event devices to get the current utilization of
>>>>>> @@ -281,24 +224,20 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>>> count = devfreq_event_get_edev_count(dev);
>>>>>> if (count < 0) {
>>>>>> dev_err(dev, "failed to get the count of devfreq-event dev\n");
>>>>>> - ret = count;
>>>>>> - goto err_regulator;
>>>>>> + return count;
>>>>>> }
>>>>>> +
>>>>>> bus->edev_count = count;
>>>>>>
>>>>>> size = sizeof(*bus->edev) * count;
>>>>>> bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
>>>>>> - if (!bus->edev) {
>>>>>> - ret = -ENOMEM;
>>>>>> - goto err_regulator;
>>>>>> - }
>>>>>> + if (!bus->edev)
>>>>>> + return -ENOMEM;
>>>>>>
>>>>>> for (i = 0; i < count; i++) {
>>>>>> bus->edev[i] = devfreq_event_get_edev_by_phandle(dev, i);
>>>>>> - if (IS_ERR(bus->edev[i])) {
>>>>>> - ret = -EPROBE_DEFER;
>>>>>> - goto err_regulator;
>>>>>> - }
>>>>>> + if (IS_ERR(bus->edev[i]))
>>>>>> + return -EPROBE_DEFER;
>>>>>> }
>>>>>>
>>>>>> /*
>>>>>> @@ -314,22 +253,15 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>>> if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
>>>>>> bus->ratio = DEFAULT_SATURATION_RATIO;
>>>>>>
>>>>>> - if (of_property_read_u32(np, "exynos,voltage-tolerance",
>>>>>> - &bus->voltage_tolerance))
>>>>>> - bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
>>>>>> -
>>>>>> return 0;
>>>>>> -
>>>>>> -err_regulator:
>>>>>> - regulator_disable(bus->regulator);
>>>>>> -
>>>>>> - return ret;
>>>>>> }
>>>>>>
>>>>>> static int exynos_bus_parse_of(struct device_node *np,
>>>>>> - struct exynos_bus *bus)
>>>>>> + struct exynos_bus *bus, bool passive)
>>>>>> {
>>>>>> struct device *dev = bus->dev;
>>>>>> + struct opp_table *opp_table;
>>>>>> + const char *vdd = "vdd";
>>>>>> struct dev_pm_opp *opp;
>>>>>> unsigned long rate;
>>>>>> int ret;
>>>>>> @@ -347,11 +279,22 @@ static int exynos_bus_parse_of(struct device_node *np,
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> + if (!passive) {
>>>>>> + opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
>>>>>> + if (IS_ERR(opp_table)) {
>>>>>> + ret = PTR_ERR(opp_table);
>>>>>> + dev_err(dev, "failed to set regulators %d\n", ret);
>>>>>> + goto err_clk;/
>>>>>> + }
>>>>>> +
>>>>>> + bus->opp_table = opp_table;
>>>>>> + }
>>>>>
>>>>> This driver has exynos_bus_parent_parse_of() function for parent devfreq device.
>>>>> dev_pm_opp_set_regulators() have to be called in exynos_bus_parent_parse_of()
>>>>> because the regulator is only used by parent devfreq device.
>>>>
>>>> exynos_bus_parse_of() is called for all devfreq devices (including
>>>> parent) and (as you've noticed) the regulator should be enabled before
>>>> enabling clock (which is done in exynos_bus_parse_of()) so adding
>>>> extra argument to exynos_bus_parse_of() (like it is done currently in
>>>> the patch)
>>>
>>> I think that this patch has still the problem about call sequence
>>> between clock and regulator as following:
>>
>> Yes, this should be fixed (though the wrong sequence between regulator
>> and clock handling is not introduced by the patchset itself and is present
>> in the original driver code).
>>
>>> 273 ret = clk_prepare_enable(bus->clk);
>>> 274 if (ret < 0) {
>>> 275 dev_err(dev, "failed to get enable clock\n");
>>> 276 return ret;
>>> 277 }
>>> 278
>>> 279 if (!passive) {
>>> 280 opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
>>> 281 if (IS_ERR(opp_table)) {
>>> 282 ret = PTR_ERR(opp_table);
>>> 283 dev_err(dev, "failed to set regulators %d\n", ret);
>>> 284 goto err_clk;
>>> 285 }
>>> 286
>>> 287 bus->opp_table = opp_table;
>>> 288 }
>>>
>>> makes it possible to do the setup correctly without the need
>>>> of merging both functions into one huge function (which would be more
>>>> difficult to follow than two simpler functions IMHO). Is that approach
>>>> acceptable or do you prefer one big function?
>>>
>>> Actually, I don't force to make one function for both
>>> exynos_bus_parse_of() and exynos_bus_parent_parse_of().
>>>
>>> If we just keep this code, dev_pm_opp_set_regulators()
>>> should be handled in exynos_bus_parent_parse_of()
>>> because only parent devfreq device controls the regulator.
>>
>> Could your please explain rationale for this requirement (besides
>> function name)?
>
> OK. I hope to satisfy the following requirements:
>
> 1. Fix the sequence problem between clock and regulator for enabling them.
> 2. dev_pm_opp_set_regulator() have to be handled in exynos_bus_parent_parse_of()
> instead of exynos_bus_parse_of() for only parent devfreq device.
> 3. exynos_bus_parse_of() have to handle the only common properties
> of both parent devfreq device and passive devfreq device.
>
>>
>> The patch adds 'bool passive' argument (which is set to false for
>> parent devfreq device and true for child devfreq device) to
>> exynos_bus_parse_of() (which is called for *all* devfreq devices
>
> As I menteiond, exynos_bus_parse_of have to handle the only common
> properties of both parent device and passive device.
>
> I gathered the properties for parent device into exynos_bus_parent_parse_of()
> This way using 'bool passive' argument is not proper in exynos_bus_parse_of().
>
>
>> and is called before exynos_bus_parent_parse_of()) and there is
>> no hard requirement to call dev_pm_opp_set_regulators() in
>> exynos_bus_parent_parse_of() so after only changing the ordering
>> between regulator and clock handling the setup code should be
>> correct.
>>
>> [ Please note that this patch moves parent/child detection before
>> exynos_bus_parse_of() call. ]
>>
>>> In order to keep the two functions, maybe have to change
>>> the call the sequence between exynos_bus_parse_of() and
>>> exynos_bus_parent_parse_of().
>>
>> Doesn't seem to be needed, care to explain it more?
>
> In order to fix the sequence problem between clock and regulator
> with dev_pm_opp_set_regualtor() and want to keep two functions
> (exynos_bus_parent_parse_of() and exynos_bus_parse_of()),
> have to change the call order as following and then modify
> the exception handling code when error happen.
>
> node = of_parse_phandle(dev->of_node, "devfreq", 0);
> if (node) {
> of_node_put(node);
> passive = true
> }
>
> if (!passive)
> exynos_bus_parent_parse_of()
> dev_pm_opp_set_regulator
>
> exynos_bus_parse_of()
OK. This seems like a solution.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
>>
>>> Once again, I don't force any fixed method. I want to fix them
>>> with correct way.
>>>
>>>>
>>>>>> +
>>>>>> /* Get the freq and voltage from OPP table to scale the bus freq */
>>>>>> ret = dev_pm_opp_of_add_table(dev);
>>>>>> if (ret < 0) {
>>>>>> dev_err(dev, "failed to get OPP table\n");
>>>>>> - goto err_clk;
>>>>>> + goto err_regulator;
>>>>>> }
>>>>>>
>>>>>> rate = clk_get_rate(bus->clk);
>>>>>> @@ -362,6 +305,7 @@ static int exynos_bus_parse_of(struct device_node *np,
>>>>>> ret = PTR_ERR(opp);
>>>>>> goto err_opp;
>>>>>> }
>>>>>> +
>>>>>> bus->curr_freq = dev_pm_opp_get_freq(opp);
>>>>>> dev_pm_opp_put(opp);
>>>>>>
>>>>>> @@ -369,6 +313,13 @@ static int exynos_bus_parse_of(struct device_node *np,
>>>>>>
>>>>>> err_opp:
>>>>>> dev_pm_opp_of_remove_table(dev);
>>>>>> +
>>>>>> +err_regulator:
>>>>>> + if (bus->opp_table) {
>>>>>> + dev_pm_opp_put_regulators(bus->opp_table);
>>>>>> + bus->opp_table = NULL;
>>>>>> + }
>>>>>
>>>>> As I mentioned above, it it wrong to call dev_pm_opp_put_regulators()
>>>>> after removing the opp_table by dev_pm_opp_of_remove_table().
>>>>>
>>>>>> +
>>>>>> err_clk:
>>>>>> clk_disable_unprepare(bus->clk);
>>>>>>
>>>>>> @@ -386,6 +337,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>>> struct exynos_bus *bus;
>>>>>> int ret, max_state;
>>>>>> unsigned long min_freq, max_freq;
>>>>>> + bool passive = false;
>>>>>>
>>>>>> if (!np) {
>>>>>> dev_err(dev, "failed to find devicetree node\n");
>>>>>> @@ -395,12 +347,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>>> bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>>>>>> if (!bus)
>>>>>> return -ENOMEM;
>>>>>> +
>>>>>> mutex_init(&bus->lock);
>>>>>> bus->dev = &pdev->dev;
>>>>>> platform_set_drvdata(pdev, bus);
>>>>>> + node = of_parse_phandle(dev->of_node, "devfreq", 0);
>>>>>> + if (node) {
>>>>>> + of_node_put(node);
>>>>>> + passive = true;
>>>>>> + }
>>>>>>
>>>>>> /* Parse the device-tree to get the resource information */
>>>>>> - ret = exynos_bus_parse_of(np, bus);
>>>>>> + ret = exynos_bus_parse_of(np, bus, passive);
>>>>>> if (ret < 0)
>>>>>> return ret;
>>>>>>
>>>>>> @@ -410,13 +368,10 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>>> goto err;
>>>>>> }
>>>>>>
>>>>>> - node = of_parse_phandle(dev->of_node, "devfreq", 0);
>>>>>> - if (node) {
>>>>>> - of_node_put(node);
>>>>>> + if (passive)
>>>>>> goto passive;
>>>>>> - } else {
>>>>>> - ret = exynos_bus_parent_parse_of(np, bus);
>>>>>> - }
>>>>>> +
>>>>>> + ret = exynos_bus_parent_parse_of(np, bus);
>>>>>>
>>>>>
>>>>> Remove unneeded blank line.
>>>>>
>>>>>> if (ret < 0)
>>>>>> goto err;
>>>>>> @@ -509,6 +464,11 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>>>
>>>>>> err:
>>>>>> dev_pm_opp_of_remove_table(dev);
>>>>>> + if (bus->opp_table) {
>>>>>> + dev_pm_opp_put_regulators(bus->opp_table);
>>>>>> + bus->opp_table = NULL;
>>>>>> + }
>>>>>> +
>>>>>
>>>>> ditto.
>>>>> Have to disable regulator after disabling the clock
>>>>> to prevent the h/w fault.
>>>>>
>>>>> I think that you should call them with following sequence:
>>>>>
>>>>> clk_disable_unprepare(bus->clk);
>>>>> if (bus->opp_table)
>>>>> dev_pm_opp_put_regulators(bus->opp_table);
>>>>> dev_pm_opp_of_remove_table(dev);
>>>>>
>>>>>> clk_disable_unprepare(bus->clk);
>>>>>>
>>>>>> return ret;
>>>>
>>>> Best regards,
>>>> --
>>>> Bartlomiej Zolnierkiewicz
>>>> Samsung R&D Institute Poland
>>>> Samsung Electronics
>>
>> Best regards,
>> --
>> Bartlomiej Zolnierkiewicz
>> Samsung R&D Institute Poland
>> Samsung Electronics
