On Tue, Sep 10, 2019 at 11:46:20AM +0200, Jean Delvare wrote:
Hi Ville,
On Mon, 2 Sep 2019 16:15:46 +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's make cea_db_offsets() a bit more convenient to use by setting the start/end offsets to zero whenever the data block collection isn't present. This makes it safe for the caller to blindly iterate the data blocks even if there are none.
Cc: Jean Delvare jdelvare@suse.de Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7b3072fc550b..e5905dc764c1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3690,6 +3690,9 @@ cea_revision(const u8 *cea) static int cea_db_offsets(const u8 *cea, int *start, int *end) {
- *start = 0;
- *end = 0;
- if (cea_revision(cea) < 3) return -ENOTSUPP;
@@ -4116,10 +4119,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) if (cea_revision(cea) >= 3) { int i, start, end;
if (cea_db_offsets(cea, &start, &end)) {start = 0;end = 0;}
cea_db_offsets(cea, &start, &end);for_each_cea_db(cea, i, start, end) { db = &cea[i];
Not sure if that's really needed. As it stands there's only one function which wants to continue after cea_db_offsets() fails, all others just bail out at that point. Now that cea_db_offsets() checks for revision >= 3, the construct above could simply become:
int i, start, end;
if (cea_db_offsets(cea, &start, &end) == 0) { for_each_cea_db(cea, i, start, end) { db = &cea[i];
which is IMHO more elegant and does not require zeroing start and end in cea_db_offsets().
I dislike indentation. Also could perhaps even move the cea_db_offsets() into the for_each_cea_db() macro so that the caller doesn't have to care about these mundane details at all.