Physical LED Design Proposal

[Ed Tanous]

On Tue, Jun 7, 2022 at 12:07 AM Jayashree D <srid.11486 at gmail.com> wrote:
>
> Hi Team,
>
> Could you please provide your suggestions on the above design for LED.
>
> Thanks,
> Jayashree
>
>
> On Fri, May 27, 2022 at 12:42 PM Jayashree D <srid.11486 at gmail.com> wrote:
>>
>> Hi Team,
>>
>> Problem Description :
>>
>> In the existing phosphor-led-sysfs design, it exposes one service per LED. Therefore, multiple services will be created for multiple GPIO pins configured for LED. To abstract this method and also to create LEDs under a single service, a new implementation is proposed.

You've kind of jumped directly to a solution without spending any
details on why this design is necessary.  What are you trying to
achieve?  Why does the existing solution not work?  You allude to
multiple services being bad, but you don't really state why, or what's
preventing that from working.  This is a section labeled problem
description, it needs to describe the problem, ideally in much more
length than your solution itself.

>>
>> Existing Implementation :
>>
>> 1. Physical Leds are defined in the device tree under "leds" section.
>> 2. Corresponding GPIO pin are defined for the physical LEDs.
>> 3. "udev rules" are used to monitor the physical LEDs.
>> 4. Once the LED in initialized in device tree, udev event will be created and it will trigger a systemd service for that LED.
>> 5. Therefore, if multiple GPIO pins are configured for LEDs, then it will create a multiple systemd services (xyz.openbmc_project.led.controller at .service) for phosphor-led-sysfs based on the LED name.
>>
>> Example :
>>
>> busctl tree xyz.openbmc_project.LED.Controller.led1
>> `-/xyz
>>   `-/xyz/openbmc_project
>>     `-/xyz/openbmc_project/led
>>       `-/xyz/openbmc_project/led/physical
>>         `-/xyz/openbmc_project/led/physical/led1
>>
>> busctl tree xyz.openbmc_project.LED.Controller.led2
>> `-/xyz
>>   `-/xyz/openbmc_project
>>     `-/xyz/openbmc_project/led
>>       `-/xyz/openbmc_project/led/physical
>>         `-/xyz/openbmc_project/led/physical/led2
>>
>>
>>
>> New Implementation :
>>
>> 1. Physical Leds are defined in the device tree under "leds" section.
>> 2. Corresponding GPIO pin are defined for the physical LEDs.
>> 3. "udev rules" are used to monitor the physical LEDs.
>> 4. Once the udev event is initialized for the LED, it will store those LED name using the script in udev instead of triggering systemd   service.
>> 5. Phosphor-led-sysfs will have a single systemd service (xyz.openbmc_project.led.controller.service) running by default at system startup.
>> 6. A dbus method call will be exposed from the service. udev will notify the LEDs detected in the driver.
>>
>> Example :
>>
>> busctl tree xyz.openbmc_project.LED.Controller
>> `-/xyz
>>   `-/xyz/openbmc_project
>>     `-/xyz/openbmc_project/led
>>       `-/xyz/openbmc_project/led/physical
>>         `-/xyz/openbmc_project/led/physical/led1
>>         `-/xyz/openbmc_project/led/physical/led2
>>
>>
>> This was already discussed in the previous mail thread. Please refer to the below link.
>> https://lists.ozlabs.org/pipermail/openbmc/2022-April/030272.html
>>
>> Please provide your suggestions on this new proposal.
>>
>>
>> Thanks
>> Jayashree