New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 795624 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

EC/consumer: Drop flush operation in struct consumer_ops

Project Member Reported by drinkcat@chromium.org, Dec 18 2017

Issue description

The consumer_ops structure in include/consumer.h has this function pointer:

	/*
	 * Flush (read) everything from the associated queue.  This call blocks
	 * until the consumer has flushed the queue.
	 */
	void (*flush)(struct consumer const *consumer);

The code was added in 2015, as part of the CL that introduced the streams abstraction:
https://chromium-review.googlesource.com/c/250941/

But it appears that the flush operation has never been used. It seems to be a remnant of the even older out stream interface: https://chromium-review.googlesource.com/216001, that never made use of the flush operation either.

Also, the implementation is oftentimes an empty function, or a variation of:

while (queue_count(consumer->queue)) {
    call queue handler; // Usually similar to calling "written" callback.
    maybe wait;
}

which might be replaceable by a generic flush function that calls the "written" callback repeatedly with a count parameter of 0 (there is one exception in chip/g/usart.c that actually flushes the HW controller, and not just the queue): but again, none of the existing users of consumer_ops structure actually call the the flush callback.

Let's remove the function pointer and implementation to save flash space, we can always restore the function if this proves to be useful in the future.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/bbb40ce21d7a972c812621091390a994c6fce4b6

commit bbb40ce21d7a972c812621091390a994c6fce4b6
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Tue Dec 19 04:32:58 2017

consumer: Remove flush operation

Nobody is calling the flush function for consumer_ops structure,
so let's remove it to save flash space, until we find a use for it.

CQ-DEPEND=CL:*529221
BRANCH=none
BUG= chromium:795624 
TEST=make buildall -j, saves from 40 to 128 bytes on some boards.

Change-Id: Iad18b30f419ccebc54a90914ec46da84b8d19601
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/826905
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/g/blob.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/include/consumer.h
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/g/usb-stream.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/stm32/usart_tx_interrupt.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/stm32/usb-stream.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/common/usb_i2c.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/stm32/usb_dwc_stream.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/stm32/usart_tx_dma.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/g/usb_spi.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/g/usb_upgrade.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/common/usb_update.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/common/queue_policies.c
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/stm32/usart_tx_dma.h
[modify] https://crrev.com/bbb40ce21d7a972c812621091390a994c6fce4b6/chip/g/usart.c

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 1 2018

Labels: merge-merged-firmware-cr50-9308.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/5808fb77d2d46fd2ea36bb7ceda319aba9450457

commit 5808fb77d2d46fd2ea36bb7ceda319aba9450457
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Feb 01 00:48:04 2018

consumer: Remove flush operation

Nobody is calling the flush function for consumer_ops structure,
so let's remove it to save flash space, until we find a use for it.

CQ-DEPEND=CL:*529221
BRANCH=none
BUG= chromium:795624 
TEST=make buildall -j, saves from 40 to 128 bytes on some boards.

Change-Id: Iad18b30f419ccebc54a90914ec46da84b8d19601
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/826905
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit bbb40ce21d7a972c812621091390a994c6fce4b6)
Reviewed-on: https://chromium-review.googlesource.com/896748
Tested-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/g/blob.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/include/consumer.h
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/g/usb-stream.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/stm32/usart_tx_interrupt.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/stm32/usb-stream.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/common/usb_i2c.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/stm32/usb_dwc_stream.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/stm32/usart_tx_dma.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/g/usb_spi.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/g/usb_upgrade.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/common/usb_update.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/common/queue_policies.c
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/stm32/usart_tx_dma.h
[modify] https://crrev.com/5808fb77d2d46fd2ea36bb7ceda319aba9450457/chip/g/usart.c

Sign in to add a comment