On Tue, Jun 7, 2022 at 9:41 PM Ed Tan= ous <edtanous@google.com> = wrote:

On Tue, J= un 7, 2022 at 12:07 AM Jayashree D <srid.11486@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@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 pin= s 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.=C2=A0 What are you trying to
achieve?=C2=A0 Why does the existing solution not work?=C2=A0 You allude to=
multiple services being bad, but you don't really state why, or what= 9;s
preventing that from working.=C2=A0 This is a section labeled problem
description, it needs to describe the problem, ideally in much more
length than your solution itself.

=C2=A0The Yosemite V2 Platform combines a Power LED a= nd a System Identification LED into a single bicolor blue-yellow LED per ho= st.
A total of 4 =C3=97 LEDs will be placed along the front edge of the= board in a grid.
The grid will be 2=C3=97rows of 2 =C3=97 LEDs to matc= h the layout of the card slots.

Based on each host status, blue or y= ellow led needs to be blink, OFF or ON. Therefore, bi-color led needs to be= paired as a group and exposed in the userspace.

<= div>Based on the existing implementation in phopshor-led-sysfs, pairing gro= ups will be difficult, since it exposes one service per LED.

Theref= ore, refactoring the phosphor-led-sysfs repo to run under a single service = and pair a group of LED which represents each host.

>>
>> Existing Implementation :
>>
>> 1. Physical Leds are defined in the device tree under "leds&q= uot; 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.control= ler@.service) for phosphor-led-sysfs based on the LED name.
>>
>> Example :
>>
>> busctl tree xyz.openbmc_project.LED.Controller.led1
>> `-/xyz
>>=C2=A0 =C2=A0`-/xyz/openbmc_project
>>=C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physical
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physic= al/led1
>>
>> busctl tree xyz.openbmc_project.LED.Controller.led2
>> `-/xyz
>>=C2=A0 =C2=A0`-/xyz/openbmc_project
>>=C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physical
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physic= al/led2
>>
>>
>>
>> New Implementation :
>>
>> 1. Physical Leds are defined in the device tree under "leds&q= uot; 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 t= hose LED name using the script in udev instead of triggering systemd=C2=A0 = =C2=A0service.
>> 5. Phosphor-led-sysfs will have a single systemd service (xyz.open= bmc_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
>>=C2=A0 =C2=A0`-/xyz/openbmc_project
>>=C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physical
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physic= al/led1
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0`-/xyz/openbmc_project/led/physic= al/led2
>>
>>
>> This was already discussed in the previous mail thread. Please ref= er to the below link.
>> https://lists.ozlabs.org/p= ipermail/openbmc/2022-April/030272.html
>>
>> Please provide your suggestions on this new proposal.
>>
>>
>> Thanks
>> Jayashree