[RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[Doug Berger]

Commit 52cdbdd49853 ("driver core: correct device's shutdown
order") allowed for proper sequencing of the gpio-keys device
shutdown callbacks by moving the device to the end of the
devices_kset list at probe which was delayed by child
dependencies.

However, commit 722e5f2b1eec ("driver core: Partially revert
"driver core: correct device's shutdown order"") removed this
portion of that commit causing a reversion in the gpio-keys
behavior which can prevent waking from shutdown.

This RFC is an attempt to find a better solution for properly
creating gpio-keys device links to ensure its suspend/resume and
shutdown services are invoked before those of its suppliers.

The first patch here is pretty brute force but simple and has
the advantage that it should be easily backportable to the
versions where the regression first occurred.

The second patch is perhaps better in spirit though still a bit
inelegant, but it can only be backported to versions of the
kernel that contain the commit in its 'Fixes:' tag. That isn't
really a valid 'Fixes:' tag since that commit did not cause the
regression, but it does represent how far the patch could be
backported.

Both commits shouldn't really exist in the same kernel so the
third patch reverts the first in an attempt to make that clear
(though it may be a source of confusion for some).

Hopefully someone with a better understanding of device links
will see a less intrusive way to automatically capture these
dependencies for parent device drivers that implement the
functions of child node devices.

Doug Berger (3):
  input: gpio-keys - use device_pm_move_to_tail
  input: gpio-keys - add device links of children
  Revert "input: gpio-keys - use device_pm_move_to_tail"

 drivers/input/keyboard/gpio_keys.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.34.1

[RFC PATCH 1/3] input: gpio-keys - use device_pm_move_to_tail

[Doug Berger]

The gpio-keys device driver implements the functionality of its
child nodes which do not receive dedicated drivers. This means
it should inherit the dependencies of these child nodes and
their effects on suspend/resume and shutdown order.

This commit exposes the device_pm_move_to_tail function to
allow the driver to move itself to the end of the lists upon a
successful probe to allow proper sequencing when other methods
are not available.

Fixes: 722e5f2b1eec ("driver core: Partially revert "driver core: correct device's shutdown order"")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/base/core.c                | 1 +
 drivers/input/keyboard/gpio_keys.c | 2 ++
 include/linux/device.h             | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 6878dfcbf0d6..8385df4d9677 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -428,6 +428,7 @@ void device_pm_move_to_tail(struct device *dev)
 	device_pm_unlock();
 	device_links_read_unlock(idx);
 }
+EXPORT_SYMBOL_GPL(device_pm_move_to_tail);
 
 #define to_devlink(dev)	container_of((dev), struct device_link, link_dev)
 
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index c42f86ad0766..0516c6279d8a 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -810,6 +810,8 @@ static int gpio_keys_probe(struct platform_device *pdev)
 	int i, error;
 	int wakeup = 0;
 
+	device_pm_move_to_tail(dev);
+
 	if (!pdata) {
 		pdata = gpio_keys_get_devtree_pdata(dev);
 		if (IS_ERR(pdata))
diff --git a/include/linux/device.h b/include/linux/device.h
index 1508e637bb26..dad40bd45509 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1082,6 +1082,7 @@ void device_link_del(struct device_link *link);
 void device_link_remove(void *consumer, struct device *supplier);
 void device_links_supplier_sync_state_pause(void);
 void device_links_supplier_sync_state_resume(void);
+void device_pm_move_to_tail(struct device *dev);
 
 extern __printf(3, 4)
 int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
-- 
2.34.1

[RFC PATCH 2/3] input: gpio-keys - add device links of children

[Doug Berger]

Since the child nodes of gpio-keys are implemented by the
gpio-keys device driver, that driver should explicitly create
the appropriate device links to support proper device power
management and shutdown sequencing.

Fixes: f9aa460672c9 ("driver core: Refactor fw_devlink feature")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 0516c6279d8a..7a0dcfeb02dc 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -865,6 +865,7 @@ static int gpio_keys_probe(struct platform_device *pdev)
 
 	for (i = 0; i < pdata->nbuttons; i++) {
 		const struct gpio_keys_button *button = &pdata->buttons[i];
+		struct fwnode_link *link;
 
 		if (!dev_get_platdata(dev)) {
 			child = device_get_next_child_node(dev, child);
@@ -882,6 +883,12 @@ static int gpio_keys_probe(struct platform_device *pdev)
 			fwnode_handle_put(child);
 			return error;
 		}
+		if (child) {
+			list_for_each_entry(link, &child->suppliers, c_hook) {
+				device_link_add(dev, link->supplier->dev,
+						DL_FLAG_AUTOREMOVE_CONSUMER);
+			}
+		}
 
 		if (button->wakeup)
 			wakeup = 1;
-- 
2.34.1

[RFC PATCH 3/3] Revert "input: gpio-keys - use device_pm_move_to_tail"

[Doug Berger]

This reverts commit 2569873096f7eb1acf63624e9772d82b23923bf4.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/base/core.c                | 1 -
 drivers/input/keyboard/gpio_keys.c | 2 --
 include/linux/device.h             | 1 -
 3 files changed, 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8385df4d9677..6878dfcbf0d6 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -428,7 +428,6 @@ void device_pm_move_to_tail(struct device *dev)
 	device_pm_unlock();
 	device_links_read_unlock(idx);
 }
-EXPORT_SYMBOL_GPL(device_pm_move_to_tail);
 
 #define to_devlink(dev)	container_of((dev), struct device_link, link_dev)
 
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 7a0dcfeb02dc..e5af0a254a3f 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -810,8 +810,6 @@ static int gpio_keys_probe(struct platform_device *pdev)
 	int i, error;
 	int wakeup = 0;
 
-	device_pm_move_to_tail(dev);
-
 	if (!pdata) {
 		pdata = gpio_keys_get_devtree_pdata(dev);
 		if (IS_ERR(pdata))
diff --git a/include/linux/device.h b/include/linux/device.h
index dad40bd45509..1508e637bb26 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1082,7 +1082,6 @@ void device_link_del(struct device_link *link);
 void device_link_remove(void *consumer, struct device *supplier);
 void device_links_supplier_sync_state_pause(void);
 void device_links_supplier_sync_state_resume(void);
-void device_pm_move_to_tail(struct device *dev);
 
 extern __printf(3, 4)
 int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
-- 
2.34.1

Re: [RFC PATCH 3/3] Revert "input: gpio-keys - use device_pm_move_to_tail"

[Greg Kroah-Hartman]

On Thu, Apr 27, 2023 at 03:16:25PM -0700, Doug Berger wrote:
> This reverts commit 2569873096f7eb1acf63624e9772d82b23923bf4.

You have to give a reason why you are reverting it please...

thanks,

greg k-h

Re: [RFC PATCH 3/3] Revert "input: gpio-keys - use device_pm_move_to_tail"

[Rafael J. Wysocki]

On Fri, Apr 28, 2023 at 6:47 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Apr 27, 2023 at 03:16:25PM -0700, Doug Berger wrote:
> > This reverts commit 2569873096f7eb1acf63624e9772d82b23923bf4.
>
> You have to give a reason why you are reverting it please...

Also, the commit ID above doesn't match any commits in the mainline.

Re: [RFC PATCH 3/3] Revert "input: gpio-keys - use device_pm_move_to_tail"

[Doug Berger]

On 4/28/2023 4:40 AM, Rafael J. Wysocki wrote:
> On Fri, Apr 28, 2023 at 6:47 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>>
>> On Thu, Apr 27, 2023 at 03:16:25PM -0700, Doug Berger wrote:
>>> This reverts commit 2569873096f7eb1acf63624e9772d82b23923bf4.
>>
>> You have to give a reason why you are reverting it please...
> 
> Also, the commit ID above doesn't match any commits in the mainline.

Apologies. I attempted to explain this in the cover letter, but as 
anticipated it caused confusion. The relevant text was:
   Both commits shouldn't really exist in the same kernel so the
   third patch reverts the first in an attempt to make that clear
   (though it may be a source of confusion for some).

This commit ID is the ID of the first patch of this set in my tree. It 
slipped my mind that of course that wouldn't be conveyed through 
send-mail :). D'oh!

To be clear, this is really a "straw man" request for comment with hope 
of finding a more elegant solution to the issue.

Thanks,
     Doug

Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[Saravana Kannan]

On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
>
> Commit 52cdbdd49853 ("driver core: correct device's shutdown
> order") allowed for proper sequencing of the gpio-keys device
> shutdown callbacks by moving the device to the end of the
> devices_kset list at probe which was delayed by child
> dependencies.
>
> However, commit 722e5f2b1eec ("driver core: Partially revert
> "driver core: correct device's shutdown order"") removed this
> portion of that commit causing a reversion in the gpio-keys
> behavior which can prevent waking from shutdown.
>
> This RFC is an attempt to find a better solution for properly
> creating gpio-keys device links to ensure its suspend/resume and
> shutdown services are invoked before those of its suppliers.
>
> The first patch here is pretty brute force but simple and has
> the advantage that it should be easily backportable to the
> versions where the regression first occurred.

We really shouldn't be calling device_pm_move_to_tail() in drivers
because device link uses device_pm_move_to_tail() for ordering too.
And it becomes a "race" between device links and when the driver calls
device_pm_move_to_tail() and I'm not sure we'll get the same ordering
every time.

>
> The second patch is perhaps better in spirit though still a bit
> inelegant, but it can only be backported to versions of the
> kernel that contain the commit in its 'Fixes:' tag. That isn't
> really a valid 'Fixes:' tag since that commit did not cause the
> regression, but it does represent how far the patch could be
> backported.
>
> Both commits shouldn't really exist in the same kernel so the
> third patch reverts the first in an attempt to make that clear
> (though it may be a source of confusion for some).
>
> Hopefully someone with a better understanding of device links
> will see a less intrusive way to automatically capture these
> dependencies for parent device drivers that implement the
> functions of child node devices.

Can you give a summary of the issue on a real system? I took a look at
the two commits you've referenced above and it's not clear what's
still broken in the 6.3+

But I'd think that just teaching fw_devlink about some property should
be sufficient. If you are facing a real issue, have you made sure you
have fw_devlink=on (this is the default unless you turned it off in
the commandline when it had issues in the past).

-Saravana

>
> Doug Berger (3):
>   input: gpio-keys - use device_pm_move_to_tail
>   input: gpio-keys - add device links of children
>   Revert "input: gpio-keys - use device_pm_move_to_tail"
>
>  drivers/input/keyboard/gpio_keys.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> --
> 2.34.1
>

Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[Saravana Kannan]

On Mon, May 1, 2023 at 1:40 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
> >
> > Commit 52cdbdd49853 ("driver core: correct device's shutdown
> > order") allowed for proper sequencing of the gpio-keys device
> > shutdown callbacks by moving the device to the end of the
> > devices_kset list at probe which was delayed by child
> > dependencies.
> >
> > However, commit 722e5f2b1eec ("driver core: Partially revert
> > "driver core: correct device's shutdown order"") removed this
> > portion of that commit causing a reversion in the gpio-keys
> > behavior which can prevent waking from shutdown.
> >
> > This RFC is an attempt to find a better solution for properly
> > creating gpio-keys device links to ensure its suspend/resume and
> > shutdown services are invoked before those of its suppliers.
> >
> > The first patch here is pretty brute force but simple and has
> > the advantage that it should be easily backportable to the
> > versions where the regression first occurred.
>
> We really shouldn't be calling device_pm_move_to_tail() in drivers
> because device link uses device_pm_move_to_tail() for ordering too.
> And it becomes a "race" between device links and when the driver calls
> device_pm_move_to_tail() and I'm not sure we'll get the same ordering
> every time.
>
> >
> > The second patch is perhaps better in spirit though still a bit
> > inelegant, but it can only be backported to versions of the
> > kernel that contain the commit in its 'Fixes:' tag. That isn't
> > really a valid 'Fixes:' tag since that commit did not cause the
> > regression, but it does represent how far the patch could be
> > backported.
> >
> > Both commits shouldn't really exist in the same kernel so the
> > third patch reverts the first in an attempt to make that clear
> > (though it may be a source of confusion for some).
> >
> > Hopefully someone with a better understanding of device links
> > will see a less intrusive way to automatically capture these
> > dependencies for parent device drivers that implement the
> > functions of child node devices.
>
> Can you give a summary of the issue on a real system? I took a look at
> the two commits you've referenced above and it's not clear what's
> still broken in the 6.3+
>
> But I'd think that just teaching fw_devlink about some property should
> be sufficient. If you are facing a real issue, have you made sure you
> have fw_devlink=on (this is the default unless you turned it off in
> the commandline when it had issues in the past).
>

I took a closer look at how gpio-keys work and I can see why
fw_devlink doesn't pick up the GPIO dependencies. It's because the
gpio dependencies are listed under child "key-x" device nodes under
the main "gpio-keys" device tree node. fw_devlink doesn't consider
dependencies under child nodes as mandatory dependencies of the parent
node.

The main reason for this was because of how fw_devlink used to work.
But I might be able to change fw_devlink to pick this up
automatically. I need to think a bit more about this because in some
cases, ignoring those dependencies is the right thing to do. Give me a
few weeks to think through and experiment with this on my end.

-Saravana

Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[Doug Berger]

On 5/2/2023 7:18 PM, Saravana Kannan wrote:
> On Mon, May 1, 2023 at 1:40 PM Saravana Kannan <saravanak@google.com> wrote:
>>
>> On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
>>>
>>> Commit 52cdbdd49853 ("driver core: correct device's shutdown
>>> order") allowed for proper sequencing of the gpio-keys device
>>> shutdown callbacks by moving the device to the end of the
>>> devices_kset list at probe which was delayed by child
>>> dependencies.
>>>
>>> However, commit 722e5f2b1eec ("driver core: Partially revert
>>> "driver core: correct device's shutdown order"") removed this
>>> portion of that commit causing a reversion in the gpio-keys
>>> behavior which can prevent waking from shutdown.
>>>
>>> This RFC is an attempt to find a better solution for properly
>>> creating gpio-keys device links to ensure its suspend/resume and
>>> shutdown services are invoked before those of its suppliers.
>>>
>>> The first patch here is pretty brute force but simple and has
>>> the advantage that it should be easily backportable to the
>>> versions where the regression first occurred.
>>
>> We really shouldn't be calling device_pm_move_to_tail() in drivers
>> because device link uses device_pm_move_to_tail() for ordering too.
>> And it becomes a "race" between device links and when the driver calls
>> device_pm_move_to_tail() and I'm not sure we'll get the same ordering
>> every time.
>>
>>>
>>> The second patch is perhaps better in spirit though still a bit
>>> inelegant, but it can only be backported to versions of the
>>> kernel that contain the commit in its 'Fixes:' tag. That isn't
>>> really a valid 'Fixes:' tag since that commit did not cause the
>>> regression, but it does represent how far the patch could be
>>> backported.
>>>
>>> Both commits shouldn't really exist in the same kernel so the
>>> third patch reverts the first in an attempt to make that clear
>>> (though it may be a source of confusion for some).
>>>
>>> Hopefully someone with a better understanding of device links
>>> will see a less intrusive way to automatically capture these
>>> dependencies for parent device drivers that implement the
>>> functions of child node devices.
>>
>> Can you give a summary of the issue on a real system? I took a look at
>> the two commits you've referenced above and it's not clear what's
>> still broken in the 6.3+
>>
>> But I'd think that just teaching fw_devlink about some property should
>> be sufficient. If you are facing a real issue, have you made sure you
>> have fw_devlink=on (this is the default unless you turned it off in
>> the commandline when it had issues in the past).
>>
> 
> I took a closer look at how gpio-keys work and I can see why
> fw_devlink doesn't pick up the GPIO dependencies. It's because the
> gpio dependencies are listed under child "key-x" device nodes under
> the main "gpio-keys" device tree node. fw_devlink doesn't consider
> dependencies under child nodes as mandatory dependencies of the parent
> node.
> 
> The main reason for this was because of how fw_devlink used to work.
> But I might be able to change fw_devlink to pick this up
> automatically. I need to think a bit more about this because in some
> cases, ignoring those dependencies is the right thing to do. Give me a
> few weeks to think through and experiment with this on my end.
Thank you for taking a deeper look at gpio-keys, and for getting through 
the gobblety-gook description in my cover-letter ;).

Yes, the dependencies of children are not automatically inherited by 
their parents and it is not clear to me whether or not that should 
change, but it definitely creates a problem for the sequencing of 
gpio-keys device callbacks.

I initially prepared the second patch as a way to explicitly create 
device links specifically for the gpio-keys device from these child 
dependencies as a work around for the fw_devlinks being dropped. I don't 
really consider this a viable patch which is why I made it an RFC, but I 
hoped it would highlight the issue.

However, the regression actually occurs in v4.18 and fw_devlink isn't 
introduced until v5.7 so it is desirable to think about solutions that 
could be backported to older versions. This is why I provided the first 
patch for discussion. Again, it is not a desirable solution just an 
illustration what could be easily backported to restore the gpio-keys 
behavior prior to commit 722e5f2b1eec ("driver core: Partially revert
"driver core: correct device's shutdown order"") without affecting other 
devices.

Thanks again for your willingness to take the time to think this through,
     Doug

> 
> -Saravana

Re: [RFC PATCH 0/3] input: gpio-keys - fix pm ordering

[Saravana Kannan]

On Wed, May 3, 2023 at 3:21 PM Doug Berger <opendmb@gmail.com> wrote:
>
> On 5/2/2023 7:18 PM, Saravana Kannan wrote:
> > On Mon, May 1, 2023 at 1:40 PM Saravana Kannan <saravanak@google.com> wrote:
> >>
> >> On Thu, Apr 27, 2023 at 3:18 PM Doug Berger <opendmb@gmail.com> wrote:
> >>>
> >>> Commit 52cdbdd49853 ("driver core: correct device's shutdown
> >>> order") allowed for proper sequencing of the gpio-keys device
> >>> shutdown callbacks by moving the device to the end of the
> >>> devices_kset list at probe which was delayed by child
> >>> dependencies.
> >>>
> >>> However, commit 722e5f2b1eec ("driver core: Partially revert
> >>> "driver core: correct device's shutdown order"") removed this
> >>> portion of that commit causing a reversion in the gpio-keys
> >>> behavior which can prevent waking from shutdown.
> >>>
> >>> This RFC is an attempt to find a better solution for properly
> >>> creating gpio-keys device links to ensure its suspend/resume and
> >>> shutdown services are invoked before those of its suppliers.
> >>>
> >>> The first patch here is pretty brute force but simple and has
> >>> the advantage that it should be easily backportable to the
> >>> versions where the regression first occurred.
> >>
> >> We really shouldn't be calling device_pm_move_to_tail() in drivers
> >> because device link uses device_pm_move_to_tail() for ordering too.
> >> And it becomes a "race" between device links and when the driver calls
> >> device_pm_move_to_tail() and I'm not sure we'll get the same ordering
> >> every time.
> >>
> >>>
> >>> The second patch is perhaps better in spirit though still a bit
> >>> inelegant, but it can only be backported to versions of the
> >>> kernel that contain the commit in its 'Fixes:' tag. That isn't
> >>> really a valid 'Fixes:' tag since that commit did not cause the
> >>> regression, but it does represent how far the patch could be
> >>> backported.
> >>>
> >>> Both commits shouldn't really exist in the same kernel so the
> >>> third patch reverts the first in an attempt to make that clear
> >>> (though it may be a source of confusion for some).
> >>>
> >>> Hopefully someone with a better understanding of device links
> >>> will see a less intrusive way to automatically capture these
> >>> dependencies for parent device drivers that implement the
> >>> functions of child node devices.
> >>
> >> Can you give a summary of the issue on a real system? I took a look at
> >> the two commits you've referenced above and it's not clear what's
> >> still broken in the 6.3+
> >>
> >> But I'd think that just teaching fw_devlink about some property should
> >> be sufficient. If you are facing a real issue, have you made sure you
> >> have fw_devlink=on (this is the default unless you turned it off in
> >> the commandline when it had issues in the past).
> >>
> >
> > I took a closer look at how gpio-keys work and I can see why
> > fw_devlink doesn't pick up the GPIO dependencies. It's because the
> > gpio dependencies are listed under child "key-x" device nodes under
> > the main "gpio-keys" device tree node. fw_devlink doesn't consider
> > dependencies under child nodes as mandatory dependencies of the parent
> > node.
> >
> > The main reason for this was because of how fw_devlink used to work.
> > But I might be able to change fw_devlink to pick this up
> > automatically. I need to think a bit more about this because in some
> > cases, ignoring those dependencies is the right thing to do. Give me a
> > few weeks to think through and experiment with this on my end.
> Thank you for taking a deeper look at gpio-keys, and for getting through
> the gobblety-gook description in my cover-letter ;).
>
> Yes, the dependencies of children are not automatically inherited by
> their parents and it is not clear to me whether or not that should
> change, but it definitely creates a problem for the sequencing of
> gpio-keys device callbacks.
>
> I initially prepared the second patch as a way to explicitly create
> device links specifically for the gpio-keys device from these child
> dependencies as a work around for the fw_devlinks being dropped. I don't
> really consider this a viable patch which is why I made it an RFC, but I
> hoped it would highlight the issue.

Thanks. It definitely made it easier for me to get to the root of the
problem/omission.

> However, the regression actually occurs in v4.18 and fw_devlink isn't
> introduced until v5.7 so it is desirable to think about solutions that
> could be backported to older versions.

For older versions, if they have device link support, I'd recommend
creating device links and letting that take care of it.

Also, if you use the right GPIO APIs, at least on newer kernels Linus
W was looking into creating device links automatically from the GPIO
framework level.

So maybe you can backport some variant of that into the older kernels
and leave it to fw_devlink on the newer ones.

-Saravana

> This is why I provided the first
> patch for discussion. Again, it is not a desirable solution just an
> illustration what could be easily backported to restore the gpio-keys
> behavior prior to commit 722e5f2b1eec ("driver core: Partially revert
> "driver core: correct device's shutdown order"") without affecting other
> devices.
>
> Thanks again for your willingness to take the time to think this through,
>      Doug
>
> >
> > -Saravana
>